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

nfs: add support for clients in the StorageClass #3895

Merged
merged 1 commit into from
Jul 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions e2e/nfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,38 @@ var _ = Describe("nfs", func() {
}
})

By("create a storageclass with a restricted set of clients allowed to mount it", func() {
clientExample := "192.168.49.29"
err := createNFSStorageClass(f.ClientSet, f, false, map[string]string{
"clients": clientExample,
})
if err != nil {
framework.Failf("failed to create NFS storageclass: %v", err)
}
pvc, err := loadPVC(pvcPath)
if err != nil {
framework.Failf("Could not create PVC: 1 %v", err)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Dont we need to create PVC here and ensure its bound?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. You are right. I am still experimenting with this code and unfortunately the only easy way to test it is to send it to the PR. Thanks for the review but it isn't ready yet. I'll post it when the code is closer to being ready.

pvc.Namespace = f.UniqueName
err = createPVCAndvalidatePV(f.ClientSet, pvc, deployTimeout)
if err != nil {
framework.Failf("failed to create PVC: %v", err)
}

if !checkExports(f, "my-nfs", clientExample) {
framework.Failf("failed in testing exports")
}

err = deletePVCAndValidatePV(f.ClientSet, pvc, deployTimeout)
if err != nil {
framework.Failf("failed to delete PVC: %v", err)
}
err = deleteResource(nfsExamplePath + "storageclass.yaml")
if err != nil {
framework.Failf("failed to delete NFS storageclass: %v", err)
}
})

By("create a PVC and bind it to an app", func() {
err := createNFSStorageClass(f.ClientSet, f, false, nil)
if err != nil {
Expand Down
101 changes: 101 additions & 0 deletions e2e/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -1743,3 +1743,104 @@ func getConfigFile(filename, preferred, fallback string) string {

return configFile
}

type nfsExportsFSAL struct {
Name string `json:"name"`
UserID string `json:"user_id"`
FSName string `json:"fs_name"`
}

type nfsExportsClients struct {
Addresses []string `json:"addresses"`
AccessType string `json:"access_type"`
Squash string `json:"squash"`
}

type cephNFSExport struct {
ExportID int `json:"export_id"`
Path string `json:"path"`
ClusterID string `json:"cluster_id"`
Pseudo string `json:"pseudo"`
AccessType string `json:"access_type"`
Squash string `json:"squash"`
SecurityLabel bool `json:"security_label"`
Protocols []int `json:"protocols"`
Transports []string `json:"transports"`
FSAL nfsExportsFSAL `json:"fsal"`
Clients []nfsExportsClients `json:"clients"`
SecTypes []string `json:"secTypes"`
}

// Get list of exports for a cluster_id.
func listExports(f *framework.Framework, clusterID string) (*[]cephNFSExport, error) {
var exportList []cephNFSExport

stdout, stdErr, err := execCommandInToolBoxPod(
f,
"ceph nfs export ls "+clusterID+" --detailed",
rookNamespace)
if err != nil {
return nil, err
}
if stdErr != "" {
return nil, fmt.Errorf("error listing exports in clusterID %v", stdErr)
}

err = json.Unmarshal([]byte(stdout), &exportList)
if err != nil {
return nil, err
}

return &exportList, nil
}

// Check the export for a listed ip address and confirm that the export has
// been setup correctly.
func checkExports(f *framework.Framework, clusterID, clientString string) bool {
exportList, err := listExports(f, clusterID)
if err != nil {
framework.Logf("failed to fetch list of exports: %v", err)

return false
}

found := false
for i := 0; i < len(*exportList); i++ {
export := (*exportList)[i]
for _, client := range export.Clients {
for _, address := range client.Addresses {
if address == clientString {
found = true

break
}
}
if found {
if client.AccessType != "rw" {
framework.Logf("Unexpected value for client AccessType: %s", client.AccessType)

return false
}

break
}
}
if found {
if export.AccessType != "none" {
framework.Logf("Unexpected value for default AccessType: %s", export.AccessType)

return false
}

break
}
}

if !found {
framework.Logf("Could not find the configured clients in the list of exports")

return false
}

return true
}
6 changes: 6 additions & 0 deletions examples/nfs/storageclass.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -51,5 +51,11 @@ parameters:
# This option is available with Ceph v17.2.6 and newer.
# secTypes: <sectype-list>

# (optional) The clients parameter in the storage class is used to limit
# access to the export to the set of hostnames, networks or ip addresses
# specified. The <client-list> is a comma delimited string,
# for example: "192.168.0.10,192.168.1.0/8"
# clients: <client-list>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a chance we need these IP's to be ever changed on the existing PVC?

Copy link
Member Author

Choose a reason for hiding this comment

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

There could be requirements to change the ip addresses in the figure. There isn't an easy solution for that and will need users to use a new storageclass with the new set of ips. Unfortunately that is a limitation of this particular design.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it possible to move this configuration to the configmap here https://github.com/ceph/ceph-csi/blob/devel/deploy/csi-config-map-sample.yaml#L68?

Copy link
Member

Choose a reason for hiding this comment

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

Not sure how practical that is. Changing the client list after the fact is not something that can (easily) be done through Ceph Mgr once the NFS-export has been created. As the StorageClass parameters are immutable, it shows the right behavior (and difficulties for changing options).

The current design allows creation of different StorageClasses with different client-lists. It becomes ugly and more error prone if that is placed in a ConfigMap.


reclaimPolicy: Delete
allowVolumeExpansion: true
5 changes: 5 additions & 0 deletions internal/nfs/controller/volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ func (nv *NFSVolume) CreateExport(backend *csi.Volume) error {
nfsCluster := backend.VolumeContext["nfsCluster"]
path := backend.VolumeContext["subvolumePath"]
secTypes := backend.VolumeContext["secTypes"]
clients := backend.VolumeContext["clients"]

err := nv.setNFSCluster(nfsCluster)
if err != nil {
Expand All @@ -157,6 +158,10 @@ func (nv *NFSVolume) CreateExport(backend *csi.Volume) error {
}
}

if clients != "" {
export.ClientAddr = strings.Split(clients, ",")
}

_, err = nfsa.CreateCephFSExport(export)
switch {
case err == nil:
Expand Down