Skip to content

Added an API call to get all NFS servers and Renamed package to fix linting issue#94

Merged
samihan-dell merged 15 commits into
mainfrom
bugfix-1808-nfs-label
Jul 7, 2025
Merged

Added an API call to get all NFS servers and Renamed package to fix linting issue#94
samihan-dell merged 15 commits into
mainfrom
bugfix-1808-nfs-label

Conversation

@samihan-dell

@samihan-dell samihan-dell commented Jun 26, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

  • Added a API call to get all NFS servers. This will be used to avoid incorrectly labelling nodes for NFS protocol by Unity CSI driver.
  • Renamed package to fix golangci-lint check error

GitHub Issues

List the GitHub issues impacted by this PR:

GitHub Issue #
https://github.com/dell/csm/issues/1808

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?

Changes are getting used in: dell/csi-unity#321

  • Installed Driver with 1 NFS Enabled Array and nfs label applies as expected and able to provision volumes.
  • Installed Driver with 1 NFS Disabled Array and nfs label not applies as expected and able to provision volumes.
  • Installed Driver with Multi-Array (Array1: NFS Disabled & Array2: NFS Enabled):
    • NFS label not added for Array1 to csinode.
    • NFS label added for Array2 to csinode.
    • Provisioning failed for NFS volumes for Array1.
    • Able to provision for NFS volumes for Array2.
  • Upgrade test from 2.14 to latest driver
    • Provisioning failed for NFS volumes.
    • Provisioning worked for non-NFS volumes.
    • Tested against non-NFS volumes created before the upgrade:
      • Validated volume can be re-mounted to the same node as well as a different node.
      • Validated Snapshot:
        • Write data to the volume.
        • Validated that a snapshot can be taken.
        • Validated volume creation from snapshot.
      • Validated that volume cloning is possible.
        • Validated that the cloned volume can be mounted.

@samihan-dell samihan-dell changed the title Added a API call to get all NFS servers Added an API call to get all NFS servers Jun 26, 2025
@samihan-dell samihan-dell mentioned this pull request Jun 27, 2025
Merged
12 tasks
@samihan-dell samihan-dell marked this pull request as ready for review July 1, 2025 11:28
suryagupta4
suryagupta4 reviewed Jul 1, 2025
View reviewed changes
Comment thread api/apiconstants.go Outdated
@suryagupta4

suryagupta4 commented Jul 1, 2025

Copy link
Copy Markdown
Contributor

Please update copyright year as well.

@samihan-dell samihan-dell requested a review from suryagupta4 July 4, 2025 10:17
@samihan-dell

samihan-dell commented Jul 4, 2025

Copy link
Copy Markdown
Contributor Author

As the folder name is changed, below path needs to be added to ignore list for UTs:
apitypes/*
It contains struct definations.

@samihan-dell samihan-dell changed the title Added an API call to get all NFS servers Added an API call to get all NFS servers and Renamed package to fix linting issue Jul 5, 2025
@github-actions

github-actions Bot commented Jul 7, 2025

Copy link
Copy Markdown
Contributor

Merging this branch will increase overall coverage

Impacted Packages Coverage Δ 🤖
github.com/dell/gounity 90.61% (+0.09%) 👍
github.com/dell/gounity/api 91.23% (ø)
github.com/dell/gounity/apitypes 0.00% (ø)
github.com/dell/gounity/gounityutil 94.37% (+94.37%) 🌟
github.com/dell/gounity/mocks 0.00% (ø)
github.com/dell/gounity/mocks/api 0.00% (ø)
Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/dell/gounity/api/apiconstants.go 0.00% (ø) 0 0 0
github.com/dell/gounity/api/restclient.go 88.43% (ø) 121 107 14
github.com/dell/gounity/apitypes/requests.go 0.00% (ø) 1 (+1) 0 1 (+1)
github.com/dell/gounity/apitypes/response.go 0.00% (ø) 0 0 0
github.com/dell/gounity/filesystem.go 91.63% (+0.29%) 239 (+8) 219 (+8) 20 👍
github.com/dell/gounity/gounityutil/util.go 94.37% (+94.37%) 71 (+71) 67 (+67) 4 (+4) 🌟
github.com/dell/gounity/host.go 90.91% (ø) 121 110 11
github.com/dell/gounity/ipinterface.go 100.00% (ø) 12 12 0
github.com/dell/gounity/metrics.go 97.92% (ø) 48 47 1
github.com/dell/gounity/mocks/UnityClient.go 0.00% (ø) 0 0 0
github.com/dell/gounity/mocks/api/Client.go 0.00% (ø) 0 0 0
github.com/dell/gounity/snapshot.go 89.76% (ø) 127 114 13
github.com/dell/gounity/storagepool.go 100.00% (ø) 14 14 0
github.com/dell/gounity/unityclient.go 87.50% (ø) 104 91 13
github.com/dell/gounity/volume.go 88.07% (ø) 176 155 21

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/gounity/api/restclient_test.go
  • github.com/dell/gounity/filesystem_test.go
  • github.com/dell/gounity/gounityutil/util_test.go
  • github.com/dell/gounity/host_test.go
  • github.com/dell/gounity/ipinterface_test.go
  • github.com/dell/gounity/metrics_test.go
  • github.com/dell/gounity/snapshot_test.go
  • github.com/dell/gounity/unityclient_test.go
  • github.com/dell/gounity/volume_test.go

@samihan-dell samihan-dell merged commit d51270b into main Jul 7, 2025
11 of 12 checks passed
@samihan-dell samihan-dell deleted the bugfix-1808-nfs-label branch July 7, 2025 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@atye atye Awaiting requested review from atye

@alexemc alexemc Awaiting requested review from alexemc

@anathoodell anathoodell Awaiting requested review from anathoodell

@ChristianAtDell ChristianAtDell Awaiting requested review from ChristianAtDell

@Deepak-Ghivari Deepak-Ghivari Awaiting requested review from Deepak-Ghivari

@donatwork donatwork Awaiting requested review from donatwork

@EvgenyUglov EvgenyUglov Awaiting requested review from EvgenyUglov

@francis-nijay francis-nijay Awaiting requested review from francis-nijay

@JacobGros JacobGros Awaiting requested review from JacobGros

@bandak2 bandak2 Awaiting requested review from bandak2

@OliverChen557 OliverChen557 Awaiting requested review from OliverChen557

@rajendraindukuri rajendraindukuri Awaiting requested review from rajendraindukuri

@shaynafinocchiaro shaynafinocchiaro Awaiting requested review from shaynafinocchiaro shaynafinocchiaro is a code owner

@panigs7 panigs7 Awaiting requested review from panigs7

@suryagupta4 suryagupta4 Awaiting requested review from suryagupta4

2 more reviewers

@karthikk92 karthikk92 karthikk92 approved these changes

@AkshaySainiDell AkshaySainiDell AkshaySainiDell approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@samihan-dell @suryagupta4 @karthikk92 @AkshaySainiDell