Skip to content
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.

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

Adding Host Based NFS for Powerstore #450

Merged
merged 28 commits into from
Apr 1, 2025
Merged

Adding Host Based NFS for Powerstore #450

merged 28 commits into from
Apr 1, 2025

Conversation

xuluna
Copy link
Contributor

@xuluna xuluna commented Mar 24, 2025

Description

Adding host based NFS feature for Powerstore.

GitHub Issues

List the GitHub issues impacted by this PR:

GitHub Issue #
dell/csm#1742

Checklist:

  • I have performed a self-review of my own code to ensure there are no formatting, vetting, linting, or security issues
  • I have verified that new and existing unit tests pass locally with my changes
  • I have not allowed coverage numbers to degenerate
  • I have maintained at least 90% code coverage
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • Backward compatibility is not broken

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Please also list any relevant details for your test configuration

  • Unit tests as shown below in PR checks.
  • Manual deploy of driver and verify with cert-csi volume provisioning.

@xuluna xuluna marked this pull request as ready for review March 25, 2025 18:52
@xuluna xuluna changed the title HBNFS Adding Host Based NFS for Powerstore Mar 25, 2025
VolumeCapability: &csi.VolumeCapability{
AccessType: &csi.VolumeCapability_Mount{
Mount: &csi.VolumeCapability_MountVolume{
MountFlags: []string{"rw"},
Copy link
Contributor

Choose a reason for hiding this comment

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

question: Should this be configurable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think @bharathsreekanth is looking into the permission here to be configurable, and needs confirmation from PM?

@xuluna xuluna requested review from falfaroc and lukeatdell March 27, 2025 19:26
lukeatdell
lukeatdell previously approved these changes Mar 28, 2025
falfaroc
falfaroc previously approved these changes Mar 28, 2025
@xuluna xuluna dismissed stale reviews from falfaroc and lukeatdell via 149e444 March 28, 2025 20:12
falfaroc
falfaroc previously approved these changes Mar 31, 2025
abhi16394
abhi16394 previously approved these changes Mar 31, 2025
@xuluna xuluna dismissed stale reviews from abhi16394 and falfaroc via 14bb79a April 1, 2025 14:07
@xuluna xuluna force-pushed the usr/spark/hbnfs branch from 3c9baad to 14bb79a Compare April 1, 2025 14:07
@xuluna xuluna requested a review from abhi16394 April 1, 2025 14:12
@bharathsreekanth
Copy link
Contributor

@xuluna Do not merge this before I merge my PR onto this branch. Please.

# Optional: false
# Default value: None
arrayID: globalID
csi-nfs: RWX
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: Does this override the accessMode provided in PVC ?
To enhance clarity, it might be helpful to consider using a more descriptive name, such as csi-nfs-accessMode: RWX

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't change the access mode of the pvc. Just for the NFS export.

@xuluna xuluna force-pushed the usr/spark/hbnfs branch from 087accb to 08f67fb Compare April 1, 2025 20:06
Copy link

github-actions bot commented Apr 1, 2025

Merging this branch will not change overall coverage

Impacted Packages Coverage Δ 🤖
github.com/dell/csi-powerstore/cmd/csi-powerstore 0.00% (ø)
github.com/dell/csi-powerstore/mocks 0.00% (ø)
github.com/dell/csi-powerstore/pkg/common 0.00% (ø)
github.com/dell/csi-powerstore/pkg/controller 0.00% (ø)
github.com/dell/csi-powerstore/pkg/node 0.00% (ø)
github.com/dell/csi-powerstore/pkg/provider 0.00% (ø)
github.com/dell/csi-powerstore/pkg/service 0.00% (ø)

Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/dell/csi-powerstore/cmd/csi-powerstore/main.go 0.00% (ø) 0 0 0
github.com/dell/csi-powerstore/mocks/ControllerInterface.go 0.00% (ø) 0 0 0
github.com/dell/csi-powerstore/mocks/NodeInterface.go 0.00% (ø) 0 0 0
github.com/dell/csi-powerstore/pkg/common/envvars.go 0.00% (ø) 0 0 0
github.com/dell/csi-powerstore/pkg/controller/controller.go 0.00% (ø) 0 0 0
github.com/dell/csi-powerstore/pkg/node/base.go 0.00% (ø) 0 0 0
github.com/dell/csi-powerstore/pkg/node/node.go 0.00% (ø) 0 0 0
github.com/dell/csi-powerstore/pkg/node/stager.go 0.00% (ø) 0 0 0
github.com/dell/csi-powerstore/pkg/provider/provider.go 0.00% (ø) 0 0 0
github.com/dell/csi-powerstore/pkg/service/controller.go 0.00% (ø) 0 0 0
github.com/dell/csi-powerstore/pkg/service/identity.go 0.00% (ø) 0 0 0
github.com/dell/csi-powerstore/pkg/service/node.go 0.00% (ø) 0 0 0
github.com/dell/csi-powerstore/pkg/service/service.go 0.00% (ø) 0 0 0

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

Changed unit test files

  • github.com/dell/csi-powerstore/cmd/csi-powerstore/main_test.go
  • github.com/dell/csi-powerstore/pkg/provider/provider_test.go
  • github.com/dell/csi-powerstore/pkg/service/controller_test.go
  • github.com/dell/csi-powerstore/pkg/service/identity_test.go
  • github.com/dell/csi-powerstore/pkg/service/node_test.go
  • github.com/dell/csi-powerstore/pkg/service/service_test.go

@xuluna xuluna merged commit 90b936f into main Apr 1, 2025
6 checks passed
@xuluna xuluna deleted the usr/spark/hbnfs branch April 1, 2025 20:11
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.

7 participants