Skip to content

STOR-3059: Add LocalVolume tolerations e2e test for provisioning and consumption on tainted nodes#642

Merged
openshift-merge-bot[bot] merged 1 commit into
openshift:mainfrom
radeore:lv-test
Jul 16, 2026
Merged

STOR-3059: Add LocalVolume tolerations e2e test for provisioning and consumption on tainted nodes#642
openshift-merge-bot[bot] merged 1 commit into
openshift:mainfrom
radeore:lv-test

Conversation

@radeore

@radeore radeore commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add a LocalVolume e2e test that verifies tolerations work end-to-end on a tainted worker node
  • Extract shared localVolumeTolerations() and localVolumeNodeTaint() helpers for reuse with existing LocalVolume tests
  • Add consumePVWithTolerations() so consumer pods can schedule on tainted nodes without changing existing consumePV() callers

Background

This covers the openshift-tests-private scenario where a LocalVolume CR with tolerations must provision storage and allow a workload to consume the PV on a tainted node

What the test verifies

  • A worker node is tainted with localstorage=testvalue:NoSchedule
  • A LocalVolume CR with matching tolerations provisions a PV on that node
  • A consumer pod with the same toleration schedules, mounts the volume, and completes successfully

Test plan

  • Run the focused e2e test:
    hack/test-e2e.sh --suite LocalVolume --ginkgo.focus "schedules and consumes PV on tainted node"
  • Run the full LocalVolume suite:
    hack/test-e2e.sh --suite LocalVolume
  $ go test -timeout 0 ./test/e2e/... -root="$(pwd)" -kubeconfig="$KUBECONFIG" -v --ginkgo.v
.
.
.
Ran 31 of 31 Specs in 1861.465 seconds
SUCCESS! -- 31 Passed | 0 Failed | 0 Pending | 0 Skipped
--- PASS: TestE2E (1861.47s)
PASS
ok  	github.com/openshift/local-storage-operator/test/e2e	1861.486s

Summary by CodeRabbit

  • Bug Fixes
    • Improved local storage workflow on tainted nodes by honoring configured tolerations during PV consumption, including setting tolerations on the consuming pod.
  • Tests
    • Added end-to-end coverage for local volumes with tolerations: taints an additional worker node, provisions a LocalVolume/PV on the toleration-matched target, validates the PV local path, and verifies consumption and cleanup.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 15, 2026
@openshift-ci-robot

openshift-ci-robot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

@radeore: This pull request references STOR-3059 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the sub-task to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Summary

  • Add a LocalVolume e2e test that verifies tolerations work end-to-end on a tainted worker node
  • Extract shared localVolumeTolerations() and localVolumeNodeTaint() helpers for reuse with existing LocalVolume tests
  • Add consumePVWithTolerations() so consumer pods can schedule on tainted nodes without changing existing consumePV() callers

Background

This covers the openshift-tests-private scenario where a LocalVolume CR with tolerations must provision storage and allow a workload to consume the PV on a tainted node

What the test verifies

  • A worker node is tainted with localstorage=testvalue:NoSchedule
  • A LocalVolume CR with matching tolerations provisions a PV on that node
  • A consumer pod with the same toleration schedules, mounts the volume, and completes successfully

Test plan

  • Run the focused e2e test:
    hack/test-e2e.sh --suite LocalVolume --ginkgo.focus "schedules and consumes PV on tainted node"
  • Run the full LocalVolume suite:
    hack/test-e2e.sh --suite LocalVolume
 $ go test -timeout 0 ./test/e2e/... -root="$(pwd)" -kubeconfig="$KUBECONFIG" -v --ginkgo.v
.
.
.
Ran 31 of 31 Specs in 1861.465 seconds
SUCCESS! -- 31 Passed | 0 Failed | 0 Pending | 0 Skipped
--- PASS: TestE2E (1861.47s)
PASS
ok  	github.com/openshift/local-storage-operator/test/e2e	1861.486s

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 30b2c67b-4d00-4ab6-8225-5e5a99361a15

📥 Commits

Reviewing files that changed from the base of the PR and between 8f7580d and 5ac9267.

📒 Files selected for processing (2)
  • test/e2e/gomega_util.go
  • test/e2e/localvolume_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • test/e2e/gomega_util.go
  • test/e2e/localvolume_test.go

Walkthrough

The e2e utilities now support tolerations on consuming Jobs. LocalVolume tests add a tainted-node scenario that provisions a tolerations-configured volume, validates its PV, and consumes it with matching Pod tolerations.

Changes

LocalVolume tolerations

Layer / File(s) Summary
Toleration-aware PV consumption
test/e2e/gomega_util.go
consumePV delegates to consumePVWithTolerations, which applies provided tolerations to the consuming Job’s Pod.
Tainted-node LocalVolume scenario
test/e2e/localvolume_test.go
Adds disk selection, taint and toleration helpers, and an ordered e2e test covering provisioning and PV consumption on a tainted worker.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant LocalVolumeTest
  participant TaintedWorker
  participant DiskMaker
  participant ConsumingJob
  LocalVolumeTest->>TaintedWorker: Apply localVolumeNodeTaint()
  LocalVolumeTest->>DiskMaker: Create LocalVolume with localVolumeTolerations()
  DiskMaker->>TaintedWorker: Discover selected disk
  DiskMaker-->>LocalVolumeTest: Create and validate PV
  LocalVolumeTest->>ConsumingJob: Consume PV with tolerations
  LocalVolumeTest->>TaintedWorker: Restore original taints
Loading

Suggested reviewers: jsafrane, mpatlasov

🚥 Pre-merge checks | ✅ 12 | ❌ 3

❌ Failed checks (3 warnings)

Check name Status Explanation Resolution
Test Structure And Quality ⚠️ Warning The new tolerations test has a bare PV-path assertion with no failure message; the rest of the setup/cleanup and timeouts look fine. Add a descriptive failure message to the PV-path assertion (and any similar bare assertions) so failures are easier to diagnose.
Microshift Test Compatibility ⚠️ Warning The new tolerations test uses local.storage.openshift.io LocalVolume CRs and assumes a second worker node (nodeEnv[1]) with no MicroShift guard. Add a MicroShift skip/label or avoid MicroShift-unsupported APIs and multi-node assumptions before running this test there.
Single Node Openshift (Sno) Test Compatibility ⚠️ Warning The new 'localvolume with tolerations' test uses nodeEnv[1] and the suite requires >=3 worker nodes; no SNO skip/guard is present. Add a [Skipped:SingleReplicaTopology] label or a runtime single-node skip before selecting the second worker node, or remove the multi-node assumption.
✅ Passed checks (12 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the new LocalVolume e2e test for provisioning and consuming on tainted nodes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed The new Context/It titles are static literals; no generated names, node names, timestamps, or other dynamic data appear in test titles.
Topology-Aware Scheduling Compatibility ✅ Passed Test-only changes add a narrow taint/toleration path; no HA-scheduling constraints, control-plane targeting, anti-affinity, or spread rules were introduced.
Ote Binary Stdout Contract ✅ Passed The PR only adds helper/test logic; no new stdout writes appear in main/init/TestMain/BeforeSuite paths, and the touched files contain none.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed New e2e code only adds tolerations/taint logic; no IPv4 literals, IP parsing, URL building, or external-host calls were added.
No-Weak-Crypto ✅ Passed Patch only adds toleration plumbing and a tainted-node e2e test; no weak crypto, custom crypto, or secret/token comparisons were introduced.
Container-Privileges ✅ Passed The PR only adds tolerations and node-taint test logic; no privileged, host namespace, SYS_ADMIN, or explicit root/allowPrivilegeEscalation settings were introduced.
No-Sensitive-Data-In-Logs ✅ Passed New logs are generic; the added helper/test only log PV creation/consumption and a cleanup error, with no secrets, PII, tokens, or hostnames.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@openshift-ci
openshift-ci Bot requested review from jsafrane and mpatlasov July 15, 2026 20:16

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with 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.

Inline comments:
In `@test/e2e/localvolume_test.go`:
- Around line 307-313: Update the DeferCleanup closure around
waitForNodeTaintUpdate to return restoreErr directly, while preserving the
existing taint restoration logic and cleanup registration. Remove the error-only
logging if it is no longer needed, ensuring Ginkgo receives any cleanup failure.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: d058a6bd-3697-42d4-84de-fdc034a12495

📥 Commits

Reviewing files that changed from the base of the PR and between 3a28b73 and 8f7580d.

📒 Files selected for processing (2)
  • test/e2e/gomega_util.go
  • test/e2e/localvolume_test.go

Comment thread test/e2e/localvolume_test.go Outdated
@radeore

radeore commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

/jira refresh

@openshift-ci-robot

openshift-ci-robot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

@radeore: This pull request references STOR-3059 which is a valid jira issue.

Details

In response to this:

/jira refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci

openshift-ci Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

@radeore: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@gnufied

gnufied commented Jul 16, 2026

Copy link
Copy Markdown
Member

/lgtm
/approve

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jul 16, 2026
@openshift-ci

openshift-ci Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gnufied, radeore

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 16, 2026
@radeore

radeore commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

/verified by CI

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Jul 16, 2026
@openshift-ci-robot

Copy link
Copy Markdown
Contributor

@radeore: This PR has been marked as verified by CI.

Details

In response to this:

/verified by CI

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-merge-bot
openshift-merge-bot Bot merged commit aaeb662 into openshift:main Jul 16, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants