Skip to content

CON-4687: Code change to include MDcache for nfs pod to handle symbolic link - #539

Open
souvanickRepo wants to merge 1 commit into
hpe-storage:masterfrom
souvanickRepo:con_4687
Open

CON-4687: Code change to include MDcache for nfs pod to handle symbolic link#539
souvanickRepo wants to merge 1 commit into
hpe-storage:masterfrom
souvanickRepo:con_4687

Conversation

@souvanickRepo

Copy link
Copy Markdown
Contributor

Code change to include MDcache for nfs Ganesha pod to handle symbolic link as raised in escalation.
Jira Link: https://jira.storage.hpecorp.net/browse/CON-4687

Comment thread pkg/flavor/kubernetes/nfs_test.go
Comment thread pkg/flavor/kubernetes/nfs.go

@kalikivayi kalikivayi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please address the review comments.

Comment thread pkg/flavor/kubernetes/nfs.go
@souvanickRepo

souvanickRepo commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Review from Co-pilot after adding update:

ESC-17375 Re-Review: NFS ConfigMap Update Logic

Assumption: As we are introducing the update of config map, so expecting no third-party will be updating the same config map.

Date: 2026-08-03
Scope: Review of the patch captured for RWX PVC symlink/mount behavior.

Context and Accepted Assumptions

  1. Ganesha pod restart/re-initialization is guaranteed after ConfigMap update, so config reload is triggered on the cluster.
  2. The specific ConfigMap is owned and managed only by CSI controller components.
  3. Symlink and RWX mount behavior validation is performed at cluster/integration level (not unit-test level).
  4. Duplicate assertion concern was already updated in the patch.

Re-Review Verdict

No blocking findings under the above assumptions.

The change is acceptable for ESC-17375 based on current code behavior and the stated operational/test guarantees.

What Went Well / What Fixed the Issue

  • The logic changed from create-only to create-or-update for the NFS ConfigMap, ensuring corrected Ganesha config is applied even when the ConfigMap already exists.
  • The update path includes conflict-aware retry behavior, which improves reliability when multiple controller actions race on the same object.
  • The no-op guard prevents unnecessary updates when the desired Data/Labels already match, reducing churn and avoiding needless restarts.
  • With the accepted operational assumption that Ganesha is restarted/re-initialized after ConfigMap change, updated NFS settings are guaranteed to be reloaded on cluster nodes.
  • This directly addresses the typical failure mode behind ESC-17375 where corrected NFS behavior would not take effect if an existing ConfigMap were left unchanged.

In short: previously, a stale existing ConfigMap could preserve old NFS behavior; now the controller converges that ConfigMap to the desired state, and cluster restart/reload applies the fix to running RWX workloads.

What Was Re-Checked

  • ConfigMap create/update path handles create races and update conflicts with retry.
  • No-op guard avoids unnecessary update calls when Data/Labels are already in desired state.
  • Test updates validate:
    • Config content additions for NFS/Ganesha sections.
    • ConfigMap update behavior on second invocation.
    • No-op behavior on third invocation with unchanged desired state.

Remaining Non-Blocking Risk

  • Unit tests do not prove end-to-end symlink traversal semantics through a mounted RWX pod.
  • This is acceptable because the behavioral validation is explicitly covered by cluster testing for ESC-17375.

@kalikivayi kalikivayi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

Protocols = 4;
SecType = "sys";
Delegations = false;
Attr_Expiration_Time = -1;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

With these changes to the ConfigMap, can you post the startup log of the NFS Ganesha Server and ensure that there are no warnings being thrown for the new parameters that we have added. You can: parse with ganesha.nfsd -N NIV_FULL_DEBUG OR check startup logs for “unknown parameter” errors or warnings. Please post the startup log to the bug or here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

nfs.log
Co-pilot analysis against the log for parameter related changes:

From this log, no hard ConfigMap parsing/apply failure is visible.

What I see on config-related behavior:

Config parsed successfully: nfs.log:245.
Core blocks are loaded and committed without WARN/CRIT parse errors:
NFS_Core_Param commit: nfs.log:176
NFSv4 commit: nfs.log:211
MDCACHE commit: nfs.log:244
EXPORT created successfully: nfs.log:547
Parameter changes appear to be taking effect:
NFS_Protocols changes from default 3 to loaded value 2: nfs.log:138, nfs.log:174
Graceless is active (server skips grace): nfs.log:208, nfs.log:320

Comment thread pkg/flavor/kubernetes/nfs.go Outdated
…ic link issue

Signed-off-by: Souvanick <souvanick.pal.choudhury@hpe.com>
@souvanickRepo

Copy link
Copy Markdown
Contributor Author

e2e run successfully:
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants