Skip to content

Adding Host Based NFS for Powerstore#450

Merged
xuluna merged 28 commits into
mainfrom
usr/spark/hbnfs
Apr 1, 2025
Merged

Adding Host Based NFS for Powerstore#450
xuluna merged 28 commits into
mainfrom
usr/spark/hbnfs

Conversation

@xuluna

@xuluna xuluna commented Mar 24, 2025

Copy link
Copy Markdown
Contributor

Description

Adding host based NFS feature for Powerstore.

GitHub Issues

List the GitHub issues impacted by this PR:

GitHub Issue #
https://github.com/dell/csm/issues/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
Comment thread go.mod Outdated
Comment thread docker-files/dev-docker Outdated
Comment thread cmd/csi-powerstore/main_test.go Outdated
Comment thread pkg/service/controller_test.go Outdated
Comment thread Makefile Outdated
Comment thread cmd/csi-powerstore/main_test.go Outdated
Comment thread mocks/ControllerInterface.go
Comment thread pkg/service/controller.go Outdated
Comment thread pkg/service/controller.go
Comment thread pkg/service/identity.go Outdated
Comment thread pkg/service/node.go
Comment thread pkg/service/node.go
VolumeCapability: &csi.VolumeCapability{
AccessType: &csi.VolumeCapability_Mount{
Mount: &csi.VolumeCapability_MountVolume{
MountFlags: []string{"rw"},

Copy link
Copy Markdown
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
Copy Markdown
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?

Comment thread pkg/service/service.go Outdated
@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
Comment thread pkg/service/node.go
@bharathsreekanth

Copy link
Copy Markdown
Contributor

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

Comment thread pkg/node/node.go Outdated
Comment thread pkg/node/node.go Outdated
Comment thread pkg/node/stager.go Outdated
Comment thread pkg/service/controller.go
# Optional: false
# Default value: None
arrayID: globalID
csi-nfs: RWX

Copy link
Copy Markdown
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
Copy Markdown
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
@github-actions

github-actions Bot commented Apr 1, 2025

Copy link
Copy Markdown
Contributor

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
@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