Skip to content

Fix NFS mount failure on IBM Cloud by fixing retry logic and opening port 2049#15119

Merged
ebenahar merged 4 commits into
red-hat-storage:masterfrom
amr1ta:fix-nfs-mount-ibmcloud
May 13, 2026
Merged

Fix NFS mount failure on IBM Cloud by fixing retry logic and opening port 2049#15119
ebenahar merged 4 commits into
red-hat-storage:masterfrom
amr1ta:fix-nfs-mount-ibmcloud

Conversation

@amr1ta
Copy link
Copy Markdown
Contributor

@amr1ta amr1ta commented May 12, 2026

Summary

Root cause of IBM Cloud failure:

IBM Cloud VPC LoadBalancer security groups block inbound traffic by default. When the NFS LB service (rook-ceph-nfs-my-nfs-load-balancer) is created, port 2049 is not allowed through the security group, so mount requests from the external NFS client VM never reach the NFS server.

Fix:

Open inbound TCP port 2049 on IBM Cloud VPC LoadBalancer security group for the NFS LB service. IBM Cloud VPC LB security groups block inbound traffic by default — same issue that required configure_ingress_load_balancer_security_group() for ports 80/443 in #[15012(https://github.com//pull/15012)]. The rule is added automatically in create_nfs_load_balancer_service() and cleaned up in delete_nfs_load_balancer_service().

Have updated, Stale subvolume assertion logic — the old assertion assert len(stale_volumes) == 0 fails if there are pre-existing stale subvolumes from other tests or previous runs. The test should only verify that its own subvolume (new_pvc[1]) was successfully deleted, not that the entire cluster has zero stale subvolumes.

Changed to:
Log stale volumes before and after delete for debugging
Log the delete command output so failures are visible
Assert that the specific subvolume created by this test is no longer in the stale list

amr1ta and others added 2 commits May 12, 2026 10:46
The retry() calls wrapping self.con.exec_cmd() were broken in two ways:
1. exec_cmd was called immediately and its tuple result passed to retry()
   instead of passing a callable, so no retry ever occurred
2. Connection.exec_cmd() returns (retcode, stdout, stderr) and never
   raises CommandFailed, so the retry exception type never triggered

Add _mount_nfs_with_retry() helper that wraps exec_cmd in a nested
function which raises CommandFailed on non-zero retcode, and calls it
via retry properly. Replace all 5 broken call sites.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Amrita Mahapatra <49347640+amr1ta@users.noreply.github.com>
IBM Cloud VPC LoadBalancer security groups block inbound traffic by
default. The NFS LB on port 2049 needs an explicit inbound rule, same
as the ingress LB needs rules for ports 80/443 (added in red-hat-storage#15012).

Add configure_nfs_lb_security_group() that finds the VPC LB backing
the rook-ceph-nfs-my-nfs-load-balancer Service and adds an inbound
TCP 2049 rule to its security groups. Call it automatically from
create_nfs_load_balancer_service() on IBM Cloud.

Add remove_nfs_lb_security_group_rules() to clean up the rule during
teardown, called from delete_nfs_load_balancer_service() before the
Service is deleted (so the VPC LB is still present for lookup).

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Amrita Mahapatra <49347640+amr1ta@users.noreply.github.com>
@amr1ta amr1ta requested review from a team as code owners May 12, 2026 05:48
@pull-request-size pull-request-size Bot added the size/L PR that changes 100-499 lines label May 12, 2026
@amr1ta amr1ta added the Verified Mark when PR was verified and log provided label May 12, 2026
@amr1ta
Copy link
Copy Markdown
Contributor Author

amr1ta commented May 12, 2026

Assert that the specific subvolume created by the test is no longer
stale after deletion, instead of asserting zero stale subvolumes
cluster-wide. Pre-existing stale subvolumes from other tests caused
false failures. Also log the delete output and stale lists for
debuggability.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Amrita Mahapatra <49347640+amr1ta@users.noreply.github.com>
@amr1ta amr1ta requested a review from dahorak May 12, 2026 12:09
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Amrita Mahapatra <49347640+amr1ta@users.noreply.github.com>
@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented May 13, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: amr1ta, dahorak, ebenahar

The full list of commands accepted by this bot can be found 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

@ebenahar ebenahar merged commit 4d499c2 into red-hat-storage:master May 13, 2026
6 of 7 checks passed
@amr1ta
Copy link
Copy Markdown
Contributor Author

amr1ta commented May 13, 2026

/cherry-pick release-4.21

@openshift-cherrypick-robot
Copy link
Copy Markdown
Collaborator

@amr1ta: new pull request created: #15128

Details

In response to this:

/cherry-pick release-4.21

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.

@amr1ta
Copy link
Copy Markdown
Contributor Author

amr1ta commented May 13, 2026

/cherry-pick release-4.20

@openshift-cherrypick-robot
Copy link
Copy Markdown
Collaborator

@amr1ta: #15119 failed to apply on top of branch "release-4.20":

Applying: Fix NFS mount retry logic in out-cluster NFS export tests
Using index info to reconstruct a base tree...
M	tests/functional/nfs_feature/test_nfs_feature_enable_for_ODF_clusters.py
Falling back to patching base and 3-way merge...
Auto-merging tests/functional/nfs_feature/test_nfs_feature_enable_for_ODF_clusters.py
CONFLICT (content): Merge conflict in tests/functional/nfs_feature/test_nfs_feature_enable_for_ODF_clusters.py
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 Fix NFS mount retry logic in out-cluster NFS export tests

Details

In response to this:

/cherry-pick release-4.20

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.

@amr1ta
Copy link
Copy Markdown
Contributor Author

amr1ta commented May 13, 2026

/cherry-pick release-4.19

@openshift-cherrypick-robot
Copy link
Copy Markdown
Collaborator

@amr1ta: #15119 failed to apply on top of branch "release-4.19":

Applying: Fix NFS mount retry logic in out-cluster NFS export tests
Using index info to reconstruct a base tree...
M	tests/functional/nfs_feature/test_nfs_feature_enable_for_ODF_clusters.py
Falling back to patching base and 3-way merge...
Auto-merging tests/functional/nfs_feature/test_nfs_feature_enable_for_ODF_clusters.py
CONFLICT (content): Merge conflict in tests/functional/nfs_feature/test_nfs_feature_enable_for_ODF_clusters.py
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 Fix NFS mount retry logic in out-cluster NFS export tests

Details

In response to this:

/cherry-pick release-4.19

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.

amr1ta added a commit to amr1ta/ocs-ci that referenced this pull request May 13, 2026
The cherry-pick of red-hat-storage#15119 brought in test changes that import
skip_for_provider_or_client_if_ocs_version from testlib (via wildcard
from marks.py), but this mark only existed in master, not release-4.20.
Add it so the py310 collectonly check passes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Amrita Mahapatra <49347640+amr1ta@users.noreply.github.com>
amr1ta added a commit to amr1ta/ocs-ci that referenced this pull request May 13, 2026
The cherry-pick of red-hat-storage#15119 brought in test changes that import
skip_for_provider_or_client_if_ocs_version from testlib (via wildcard
from marks.py), but this mark only existed in master, not release-4.19.
Add it so the py310 collectonly check passes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Amrita Mahapatra <49347640+amr1ta@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm size/L PR that changes 100-499 lines Verified Mark when PR was verified and log provided

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants