From c2eafce5347342921990a1df1fa16c77caff6caa Mon Sep 17 00:00:00 2001 From: Ravi Kumar Date: Tue, 10 Mar 2026 11:49:22 +0530 Subject: [PATCH 1/4] V1.2.9-rc1 1. set share objective to it's root not to (/) path. --- VERSION | 2 +- pkg/client/hsclient.go | 32 +++++++++++++++++++++++--------- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/VERSION b/VERSION index d1f79a9..989aef6 100755 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -v1.2.8 +v1.2.9-rc1 diff --git a/pkg/client/hsclient.go b/pkg/client/hsclient.go index fe28f7a..c12f103 100755 --- a/pkg/client/hsclient.go +++ b/pkg/client/hsclient.go @@ -678,8 +678,8 @@ func (client *HammerspaceClient) CreateShare(ctx context.Context, log.Errorf("No task returned to monitor") } - // Set objectives on share - err = client.SetObjectives(ctx, name, "/", objectives, true) + // Set objectives on share (share-level, not root path) + err = client.SetShareObjectives(ctx, name, objectives, true) if err != nil { log.Errorf("Failed to set objectives %s, %v", objectives, err) return err @@ -762,8 +762,8 @@ func (client *HammerspaceClient) CreateShareFromSnapshot(ctx context.Context, na log.Errorf("No task returned to monitor") } - // Set objectives on share - err = client.SetObjectives(ctx, name, "/", objectives, true) + // Set objectives on share (share-level, not root path) + err = client.SetShareObjectives(ctx, name, objectives, true) if err != nil { log.Errorf("Failed to set objectives %s, %v", objectives, err) return err @@ -800,18 +800,32 @@ func (client *HammerspaceClient) CheckIfShareCreateTaskIsRunning(ctx context.Con return false, nil } -// Set objectives on a share, at the specified path, optionally clearing previously-set objectives at the path -// The path must start with a slash +// Set objectives at the share level (no path), optionally clearing existing share-level objectives. +func (client *HammerspaceClient) SetShareObjectives(ctx context.Context, shareName string, + objectives []string, + replaceExisting bool) error { + return client.SetObjectives(ctx, shareName, "", objectives, replaceExisting) +} + +// Set objectives on a share at the specified path, optionally clearing previously-set objectives. +// Path rules: +// - "" => share-level objectives (no path query param) +// - "/x" => path-level objectives (must start with a slash) func (client *HammerspaceClient) SetObjectives(ctx context.Context, shareName string, path string, objectives []string, replaceExisting bool) error { log.Debugf("Setting objectives. Share=%s, Path=%s, Objectives=%v: ", shareName, path, objectives) - // Set objectives on share at path cleared := false for _, objectiveName := range objectives { - urlPath := fmt.Sprintf("/shares/%s/objective-set?path=%s&objective-identifier=%s", - shareName, path, objectiveName) + var urlPath string + if path == "" { + urlPath = fmt.Sprintf("/shares/%s/objective-set?objective-identifier=%s", + shareName, objectiveName) + } else { + urlPath = fmt.Sprintf("/shares/%s/objective-set?path=%s&objective-identifier=%s", + shareName, path, objectiveName) + } if replaceExisting && !cleared { urlPath += "&clear-existing=true" cleared = true From d52b106374e745dc0b4ac98e0408fcdb82c833ec Mon Sep 17 00:00:00 2001 From: Ravi Kumar Date: Thu, 12 Mar 2026 22:00:24 +0530 Subject: [PATCH 2/4] V1.2.9-rc1 1. Update code to use passed mountOptions. 2. Used storage class secret value for different secret parms. --- CHANGELOG.md | 4 + docs/multi-secret-sorageclass.md | 133 ++++++++++++++++ pkg/common/hs_types.go | 4 + pkg/driver/controller.go | 266 ++++++++++++++++++++++++------- pkg/driver/node.go | 7 +- pkg/driver/node_helper.go | 2 +- pkg/driver/utils.go | 9 +- 7 files changed, 363 insertions(+), 62 deletions(-) create mode 100644 docs/multi-secret-sorageclass.md diff --git a/CHANGELOG.md b/CHANGELOG.md index b8e4326..cc42296 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- Passed mount options from the storage class to the mount command. +- Fixed an issue where objectives was being applied to share "/" instead it should directly apply to share. + ## [1.2.8] ### Added diff --git a/docs/multi-secret-sorageclass.md b/docs/multi-secret-sorageclass.md new file mode 100644 index 0000000..c6357cf --- /dev/null +++ b/docs/multi-secret-sorageclass.md @@ -0,0 +1,133 @@ +### Here are a few example YAML files to illustrate this: + +1. Secret Definitions: + +First, you need to define the secrets in your Kubernetes cluster. Let's create two secrets, hs-secret-1 and hs-secret-2, in the default namespace. + +- secret-1.yaml +```yaml +apiVersion: v1 +kind: Secret +metadata: + name: hs-secret-1 + namespace: default +type: Opaque +stringData: + username: "admin" + password: "password" + csiEndpoint: "10.10.10.10" + csiTlsVerify: "false" +``` +- secret-2.yaml +```yaml +apiVersion: v1 +kind: Secret +metadata: + name: hs-secret-2 + namespace: default +type: Opaque +stringData: + username: "user2" + password: "password2" + csiEndpoint: "10.10.10.11" + csiTlsVerify: "false" +``` + +2. Storage Class Definitions: + +Now, let's define two storage classes, each referencing a different secret. + +- hs-sc-1.yaml +```yaml +apiVersion: storage.k8s.io/v1 +kind: StorageClass +metadata: + name: hs-sc-1 +provisioner: csi.hammerspace.com +parameters: + csi.storage.k8s.io/provisioner-secret-name: hs-secret-1 + csi.storage.k8s.io/provisioner-secret-namespace: default + # Other parameters specific to your storage provisioner + fsType: "nfs" + volumeNameFormat: "pvc-%s" +reclaimPolicy: Delete +``` +- hs-sc-2.yaml +```yaml +apiVersion: storage.k8s.io/v1 +kind: StorageClass +metadata: + name: hs-sc-2 +provisioner: csi.hammerspace.com +parameters: + csi.storage.k8s.io/provisioner-secret-name: hs-secret-2 + csi.storage.k8s.io/provisioner-secret-namespace: default + # Other parameters specific to your storage provisioner + fsType: "xfs" + volumeNameFormat: "test-%s" +reclaimPolicy: Delete +``` + +3. PersistentVolumeClaim (PVC) Definitions: + +Finally, let's create PersistentVolumeClaims (PVCs) that use these storage classes. + +- hs-pvc-1.yaml +```yaml +apiVersion: v1 +kind: PersistentVolumeClaim +metadata: + name: hs-pvc-1 +spec: + storageClassName: hs-sc-1 + accessModes: + - ReadWriteMany + resources: + requests: + storage: 1Gi +``` +- hs-pvc-2.yaml + +```yaml +apiVersion: v1 +kind: PersistentVolumeClaim +metadata: + name: hs-pvc-2 +spec: + storageClassName: hs-sc-2 + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 2Gi +``` + +### Explanation: + +- Secrets: The Secret resources store the credentials needed by the CSI driver to communicate with the Hammerspace backend. The stringData field is used to store the username, password, and CSI endpoint. + +- Storage Classes: The StorageClass resources define the type of storage to be provisioned. The parameters section includes: +csi.storage.k8s.io/provisioner-secret-name: Specifies the name of the secret containing the credentials. +csi.storage.k8s.io/provisioner-secret-namespace: Specifies the namespace where the secret is located. +Other storage-specific parameters (e.g., fsType, volumeNameFormat). + +- PersistentVolumeClaims: The PersistentVolumeClaim resources request storage from the provisioner. The storageClassName field specifies which storage class to use, which in turn determines which secret will be used for provisioning. + +#### Testing the Configuration + +Apply the Secrets: +```bash +kubectl apply -f hs-secret-1.yaml +kubectl apply -f hs-secret-2.yaml +``` +Apply the Storage Classes: +```bash +kubectl apply -f hs-sc-1.yaml +kubectl apply -f hs-sc-2.yaml +``` +Apply the PVCs: +```bash +kubectl apply -f hs-pvc-1.yaml +kubectl apply -f hs-pvc-2.yaml +``` +After applying these YAML files, the CSI driver should use hs-secret-1 to provision the volume for hs-pvc-secret-1 and hs-secret-2 to provision the volume for hs-pvc-secret-2. You can verify this by inspecting the logs of the CSI driver to see which credentials were used for each provisioning operation. \ No newline at end of file diff --git a/pkg/common/hs_types.go b/pkg/common/hs_types.go index 057d911..1a969db 100644 --- a/pkg/common/hs_types.go +++ b/pkg/common/hs_types.go @@ -30,6 +30,10 @@ type HSVolumeParameters struct { CacheEnabled bool FQDN string ClientMountOptions []string + SecretName string + SecretNamespace string + CsiEndpoint string + CsiTlsVerify bool } type HSVolume struct { diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index 9815905..b2138fb 100755 --- a/pkg/driver/controller.go +++ b/pkg/driver/controller.go @@ -37,6 +37,7 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/status" + "github.com/hammer-space/csi-plugin/pkg/client" "github.com/hammer-space/csi-plugin/pkg/common" ) @@ -166,15 +167,26 @@ func parseVolParams(params map[string]string) (common.HSVolumeParameters, error) vParams.FQDN = FQDN } - clientMountOptions, exists := params["clientMountOptions"] - if exists { - vParams.ClientMountOptions = strings.Split(clientMountOptions, ",") + if params["clientMountOptions"] != "" { + vParams.ClientMountOptions = strings.Split(params["clientMountOptions"], ",") + } + + vParams.SecretName = params["secretName"] + vParams.SecretNamespace = params["secretNamespace"] + vParams.CsiEndpoint = params["csiEndpoint"] + + if params["csiTlsVerify"] != "" { + csiTlsVerify, err := strconv.ParseBool(params["csiTlsVerify"]) + if err != nil { + vParams.CsiTlsVerify = false + } + vParams.CsiTlsVerify = csiTlsVerify } return vParams, nil } -func (d *CSIDriver) ensureNFSDirectoryExists(ctx context.Context, backingShareName string, hsVolume *common.HSVolume) error { +func (d *CSIDriver) ensureNFSDirectoryExists(ctx context.Context, hsclient *client.HammerspaceClient, backingShareName string, hsVolume *common.HSVolume) error { // Check if backing share exists unlock, err := d.acquireVolumeLock(ctx, backingShareName) if err != nil { @@ -183,7 +195,7 @@ func (d *CSIDriver) ensureNFSDirectoryExists(ctx context.Context, backingShareNa } defer unlock() - backingShare, err := d.ensureBackingShareExists(ctx, backingShareName, hsVolume) + backingShare, err := d.ensureBackingShareExists(ctx, hsclient, backingShareName, hsVolume) if err != nil { return status.Errorf(codes.Internal, "%s", err.Error()) } @@ -210,10 +222,10 @@ func (d *CSIDriver) ensureNFSDirectoryExists(ctx context.Context, backingShareNa return nil } -func (d *CSIDriver) ensureShareBackedVolumeExists(ctx context.Context, hsVolume *common.HSVolume) error { +func (d *CSIDriver) ensureShareBackedVolumeExists(ctx context.Context, hsclient *client.HammerspaceClient, hsVolume *common.HSVolume) error { // Check if the Mount Volume Exists - share, err := d.hsclient.GetShare(ctx, hsVolume.Name) + share, err := hsclient.GetShare(ctx, hsVolume.Name) if err != nil { return fmt.Errorf("failed to get share: %w", err) } @@ -230,7 +242,7 @@ func (d *CSIDriver) ensureShareBackedVolumeExists(ctx context.Context, hsVolume if hsVolume.SourceSnapPath != "" { // Create from snapshot - sourceShare, err := d.hsclient.GetShare(ctx, hsVolume.SourceSnapShareName) + sourceShare, err := hsclient.GetShare(ctx, hsVolume.SourceSnapShareName) if err != nil { log.Errorf("Failed to restore from snapshot, %v", err) return status.Error(codes.Internal, common.UnknownError) @@ -238,7 +250,7 @@ func (d *CSIDriver) ensureShareBackedVolumeExists(ctx context.Context, hsVolume if sourceShare == nil { return status.Error(codes.NotFound, common.SourceSnapshotShareNotFound) } - snapshots, err := d.hsclient.GetShareSnapshots(ctx, hsVolume.SourceSnapShareName) + snapshots, err := hsclient.GetShareSnapshots(ctx, hsVolume.SourceSnapShareName) if err != nil { log.Errorf("Failed to restore from snapshot, %v", err) return status.Error(codes.Internal, common.UnknownError) @@ -249,7 +261,7 @@ func (d *CSIDriver) ensureShareBackedVolumeExists(ctx context.Context, hsVolume return status.Error(codes.NotFound, common.SourceSnapshotNotFound) } - err = d.hsclient.CreateShareFromSnapshot( + err = hsclient.CreateShareFromSnapshot( ctx, hsVolume.Name, hsVolume.Path, @@ -266,7 +278,7 @@ func (d *CSIDriver) ensureShareBackedVolumeExists(ctx context.Context, hsVolume } } else { // Share is not there, try creating a new share - err = d.hsclient.CreateShare( + err = hsclient.CreateShare( ctx, hsVolume.Name, hsVolume.Path, @@ -305,13 +317,13 @@ func (d *CSIDriver) ensureShareBackedVolumeExists(ctx context.Context, hsVolume return nil } -func (d *CSIDriver) ensureBackingShareExists(ctx context.Context, backingShareName string, hsVolume *common.HSVolume) (*common.ShareResponse, error) { - share, err := d.hsclient.GetShare(ctx, backingShareName) +func (d *CSIDriver) ensureBackingShareExists(ctx context.Context, hsclient *client.HammerspaceClient, backingShareName string, hsVolume *common.HSVolume) (*common.ShareResponse, error) { + share, err := hsclient.GetShare(ctx, backingShareName) if err != nil { return nil, status.Errorf(codes.Internal, "%s", err.Error()) } if share == nil { - err = d.hsclient.CreateShare( + err = hsclient.CreateShare( ctx, backingShareName, hsVolume.Path, @@ -324,7 +336,7 @@ func (d *CSIDriver) ensureBackingShareExists(ctx context.Context, backingShareNa if err != nil { return nil, status.Errorf(codes.Internal, "%s", err.Error()) } - share, err = d.hsclient.GetShare(ctx, backingShareName) + share, err = hsclient.GetShare(ctx, backingShareName) if err != nil { return nil, status.Errorf(codes.Internal, "%s", err.Error()) } @@ -349,7 +361,7 @@ func (d *CSIDriver) ensureBackingShareExists(ctx context.Context, backingShareNa return share, err } -func (d *CSIDriver) ensureDeviceFileExists(ctx context.Context, backingShare *common.ShareResponse, hsVolume *common.HSVolume) error { +func (d *CSIDriver) ensureDeviceFileExists(ctx context.Context, hsclient *client.HammerspaceClient, backingShare *common.ShareResponse, hsVolume *common.HSVolume) error { log.WithFields(log.Fields{ "backingShare": backingShare, "hsVolume": hsVolume, @@ -359,7 +371,7 @@ func (d *CSIDriver) ensureDeviceFileExists(ctx context.Context, backingShare *co log.Debugf("checking if file exist %s", hsVolume.Path) // Step 1: Check if file already exists in metadata - file, err := d.hsclient.GetFile(ctx, hsVolume.Path) + file, err := hsclient.GetFile(ctx, hsVolume.Path) if err != nil { return status.Errorf(codes.Internal, "%s", err.Error()) } @@ -389,7 +401,7 @@ func (d *CSIDriver) ensureDeviceFileExists(ctx context.Context, backingShare *co // Step 3: Create file from snapshot or empty if hsVolume.SourceSnapPath != "" { // Restore from snapshot - err := d.hsclient.RestoreFileSnapToDestination(ctx, hsVolume.SourceSnapPath, hsVolume.Path) + err := hsclient.RestoreFileSnapToDestination(ctx, hsVolume.SourceSnapPath, hsVolume.Path) if err != nil { log.Errorf("Failed to restore from snapshot, %v", err) return status.Error(codes.NotFound, common.UnknownError) @@ -428,7 +440,7 @@ func (d *CSIDriver) ensureDeviceFileExists(ctx context.Context, backingShare *co metadataCtx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) defer cancel() - err = d.applyObjectiveAndMetadata(metadataCtx, backingShare, hsVolume, deviceFile) + err = d.applyObjectiveAndMetadata(metadataCtx, hsclient, backingShare, hsVolume, deviceFile) if err != nil { log.Warnf("Unable to apply objective and metadata over backing share %s, device path %s: %v", backingShare.Name, deviceFile, err) } @@ -437,7 +449,7 @@ func (d *CSIDriver) ensureDeviceFileExists(ctx context.Context, backingShare *co } // ensure from hs system /share/file exist to apply objective and metadata -func (d *CSIDriver) applyObjectiveAndMetadata(ctx context.Context, backingShare *common.ShareResponse, hsVolume *common.HSVolume, deviceFile string) error { +func (d *CSIDriver) applyObjectiveAndMetadata(ctx context.Context, hsclient *client.HammerspaceClient, backingShare *common.ShareResponse, hsVolume *common.HSVolume, deviceFile string) error { b := &backoff.Backoff{ Max: 5 * time.Second, Factor: 1.5, @@ -451,7 +463,7 @@ func (d *CSIDriver) applyObjectiveAndMetadata(ctx context.Context, backingShare time.Sleep(dur) // Wait for file to exist on metadata server log.Debugf("Checking existance of file %s", hsVolume.Path) - backingFileExists, err = d.hsclient.DoesFileExist(ctx, hsVolume.Path) + backingFileExists, err = hsclient.DoesFileExist(ctx, hsVolume.Path) if err != nil { log.Warnf("Error checking file existence: %v", err) time.Sleep(time.Second) @@ -471,7 +483,8 @@ func (d *CSIDriver) applyObjectiveAndMetadata(ctx context.Context, backingShare if len(hsVolume.Objectives) > 0 { filePath := GetVolumeNameFromPath(hsVolume.Path) - err = d.hsclient.SetObjectives(ctx, backingShare.Name, filePath, hsVolume.Objectives, true) + // dont set replaceObjective to true + err = hsclient.SetObjectives(ctx, backingShare.Name, filePath, hsVolume.Objectives, false) if err != nil { log.Errorf("failed to set objectives on backing file for volume: %v\n", err) return err @@ -486,7 +499,7 @@ func (d *CSIDriver) applyObjectiveAndMetadata(ctx context.Context, backingShare return err } -func (d *CSIDriver) ensureFileBackedVolumeExists(ctx context.Context, hsVolume *common.HSVolume, backingShareName string) error { +func (d *CSIDriver) ensureFileBackedVolumeExists(ctx context.Context, hsclient *client.HammerspaceClient, hsVolume *common.HSVolume, backingShareName string) error { log.WithFields(log.Fields{ "backingShareName": backingShareName, @@ -501,12 +514,12 @@ func (d *CSIDriver) ensureFileBackedVolumeExists(ctx context.Context, hsVolume * } defer unlock() - backingShare, err := d.ensureBackingShareExists(ctx, backingShareName, hsVolume) + backingShare, err := d.ensureBackingShareExists(ctx, hsclient, backingShareName, hsVolume) if err != nil { return status.Errorf(codes.Internal, "%s", err.Error()) } log.Debugf("Backing share existed %s", backingShareName) - err = d.ensureDeviceFileExists(ctx, backingShare, hsVolume) + err = d.ensureDeviceFileExists(ctx, hsclient, backingShare, hsVolume) return err } @@ -535,6 +548,23 @@ func (d *CSIDriver) CreateVolume(ctx context.Context, req *csi.CreateVolumeReque return nil, err } + hsclient := d.hsclient + if vParams.SecretName != "" { + if vParams.CsiEndpoint == "" { + return nil, status.Errorf(codes.InvalidArgument, "csiEndpoint must be specified with secretName") + } + username, uOk := req.Secrets["username"] + password, pOk := req.Secrets["password"] + if !uOk || !pOk { + return nil, status.Errorf(codes.InvalidArgument, "username and password must be in secrets") + } + newClient, err := client.NewHammerspaceClient(vParams.CsiEndpoint, username, password, vParams.CsiTlsVerify) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to create new hsclient, %v", err) + } + hsclient = newClient + } + // Check for snapshot source specified cs := req.VolumeContentSource snap := cs.GetSnapshot() @@ -638,7 +668,7 @@ func (d *CSIDriver) CreateVolume(ctx context.Context, req *csi.CreateVolumeReque } else { log.Infof("getting free capacity from (/cntl/state) api response") // Call your function to get the free capacity from the API response here - available, err = d.hsclient.GetClusterAvailableCapacity(ctx) + available, err = hsclient.GetClusterAvailableCapacity(ctx) if err != nil { return nil, status.Error(codes.Internal, err.Error()) } @@ -662,7 +692,7 @@ func (d *CSIDriver) CreateVolume(ctx context.Context, req *csi.CreateVolumeReque } } else { // If cached objective list is nil or empty, fetch it from the API - clusterObjectiveNames, err = d.hsclient.ListObjectiveNames(ctx) + clusterObjectiveNames, err = hsclient.ListObjectiveNames(ctx) if err != nil { return nil, status.Error(codes.Internal, err.Error()) } @@ -709,7 +739,7 @@ func (d *CSIDriver) CreateVolume(ctx context.Context, req *csi.CreateVolumeReque if !fileBacked && fsType == "nfs" && vParams.MountBackingShareName != "" { // This function is called when user want new nfs share inside one base share log.Debugf("Creating share for NFS volume inside base NFS share dir %s with path %s", vParams.MountBackingShareName, hsVolume.Path) - err := d.ensureNFSDirectoryExists(ctx, backingShareName, hsVolume) + err := d.ensureNFSDirectoryExists(ctx, hsclient, backingShareName, hsVolume) if err != nil { log.Errorf("failed to ensure base NFS share (%s): %v", backingShareName, err) return nil, status.Errorf(codes.Internal, "failed to ensure base NFS share (%s): %v", backingShareName, err) @@ -720,7 +750,7 @@ func (d *CSIDriver) CreateVolume(ctx context.Context, req *csi.CreateVolumeReque } else if fileBacked { // This function will be called in case of Block and File backed share log.Debugf("Creating share for File system volume (block or files) inside base backingshare name dir %s with path %s", backingShareName, hsVolume.Path) - err = d.ensureFileBackedVolumeExists(ctx, hsVolume, backingShareName) + err = d.ensureFileBackedVolumeExists(ctx, hsclient, hsVolume, backingShareName) if err != nil { return nil, err } @@ -733,7 +763,7 @@ func (d *CSIDriver) CreateVolume(ctx context.Context, req *csi.CreateVolumeReque // Then we create snapshot of that share /pvc-csi-uuid which will be inside /k8s-nfs-share/.snapshot // Then restore the snapshot to the new created share from snapshot content source. log.Debugf("Creating share for NFS volume with path %s", hsVolume.Path) - err = d.ensureShareBackedVolumeExists(ctx, hsVolume) + err = d.ensureShareBackedVolumeExists(ctx, hsclient, hsVolume) if err != nil { return nil, err } @@ -776,14 +806,14 @@ func (d *CSIDriver) CreateVolume(ctx context.Context, req *csi.CreateVolumeReque return resp, nil } -func (d *CSIDriver) deleteFileBackedVolume(ctx context.Context, filepath string) error { +func (d *CSIDriver) deleteFileBackedVolume(ctx context.Context, hsclient *client.HammerspaceClient, filepath string) error { var exists bool - if exists, _ = d.hsclient.DoesFileExist(ctx, filepath); exists { + if exists, _ = hsclient.DoesFileExist(ctx, filepath); exists { log.Debugf("found file-backed volume to delete, %s", filepath) } // Check if file has snapshots and fail - snaps, _ := d.hsclient.GetFileSnapshots(ctx, filepath) + snaps, _ := hsclient.GetFileSnapshots(ctx, filepath) if len(snaps) > 0 { return status.Errorf(codes.FailedPrecondition, common.VolumeDeleteHasSnapshots) } @@ -824,9 +854,9 @@ func (d *CSIDriver) deleteFileBackedVolume(ctx context.Context, filepath string) return nil } -func (d *CSIDriver) deleteShareBackedVolume(ctx context.Context, share *common.ShareResponse) error { +func (d *CSIDriver) deleteShareBackedVolume(ctx context.Context, hsclient *client.HammerspaceClient, share *common.ShareResponse) error { // Check for snapshots - snaps, err := d.hsclient.GetShareSnapshots(ctx, share.Name) + snaps, err := hsclient.GetShareSnapshots(ctx, share.Name) if err != nil { return status.Errorf(codes.Internal, "%s", err.Error()) } @@ -842,7 +872,7 @@ func (d *CSIDriver) deleteShareBackedVolume(ctx context.Context, share *common.S log.Warnf("csi_delete_delay extended info, %s, should be an integer, on share %s; falling back to cluster defaults", v, share.Name) } } - err = d.hsclient.DeleteShare(ctx, share.Name, deleteDelay) + err = hsclient.DeleteShare(ctx, share.Name, deleteDelay) if err != nil { return status.Errorf(codes.Internal, "%s", err.Error()) } @@ -870,17 +900,34 @@ func (d *CSIDriver) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeReque } defer unlock() + hsclient := d.hsclient + if req.Secrets != nil { + if endpoint, ok := req.Secrets["csiEndpoint"]; ok { + username, uOk := req.Secrets["username"] + password, pOk := req.Secrets["password"] + if !uOk || !pOk { + return nil, status.Errorf(codes.InvalidArgument, "username and password must be in secrets") + } + tlsVerify, _ := strconv.ParseBool(req.Secrets["csiTlsVerify"]) + newClient, err := client.NewHammerspaceClient(endpoint, username, password, tlsVerify) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to create new hsclient, %v", err) + } + hsclient = newClient + } + } + volumeName := GetVolumeNameFromPath(volumeId) - share, err := d.hsclient.GetShare(ctx, volumeName) + share, err := hsclient.GetShare(ctx, volumeName) if err != nil { return nil, status.Errorf(codes.Internal, "%s", err.Error()) } if share == nil { // Share does not exist, may be a file-backed volume - err = d.deleteFileBackedVolume(ctx, volumeId) + err = d.deleteFileBackedVolume(ctx, hsclient, volumeId) return &csi.DeleteVolumeResponse{}, err } else { // Share exists and is a Filesystem - err = d.deleteShareBackedVolume(ctx, share) + err = d.deleteShareBackedVolume(ctx, hsclient, share) return &csi.DeleteVolumeResponse{}, err } @@ -926,15 +973,32 @@ func (d *CSIDriver) ControllerExpandVolume(ctx context.Context, req *csi.Control return nil, status.Error(codes.InvalidArgument, common.VolumeNotFound) } + hsclient := d.hsclient + if req.Secrets != nil { + if endpoint, ok := req.Secrets["csiEndpoint"]; ok { + username, uOk := req.Secrets["username"] + password, pOk := req.Secrets["password"] + if !uOk || !pOk { + return nil, status.Errorf(codes.InvalidArgument, "username and password must be in secrets") + } + tlsVerify, _ := strconv.ParseBool(req.Secrets["csiTlsVerify"]) + newClient, err := client.NewHammerspaceClient(endpoint, username, password, tlsVerify) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to create new hsclient, %v", err) + } + hsclient = newClient + } + } + volumeName := GetVolumeNameFromPath(req.GetVolumeId()) - share, _ := d.hsclient.GetShare(ctx, volumeName) + share, _ := hsclient.GetShare(ctx, volumeName) if share == nil { fileBacked = true } // Check if the specified backing share or file exists if share == nil { - backingFileExists, err := d.hsclient.DoesFileExist(ctx, req.GetVolumeId()) + backingFileExists, err := hsclient.DoesFileExist(ctx, req.GetVolumeId()) if err != nil { log.Error(err) } @@ -946,7 +1010,7 @@ func (d *CSIDriver) ControllerExpandVolume(ctx context.Context, req *csi.Control } if fileBacked { - file, err := d.hsclient.GetFile(ctx, req.GetVolumeId()) + file, err := hsclient.GetFile(ctx, req.GetVolumeId()) if file == nil || err != nil { return nil, status.Error(codes.NotFound, common.VolumeNotFound) } else { @@ -963,7 +1027,7 @@ func (d *CSIDriver) ControllerExpandVolume(ctx context.Context, req *csi.Control // if required - current > available on backend share sizeDiff := requestedSize - file.Size backingShareName := path.Base(path.Dir(req.GetVolumeId())) - backingShare, err := d.hsclient.GetShare(ctx, backingShareName) + backingShare, err := hsclient.GetShare(ctx, backingShareName) if err != nil { return nil, fmt.Errorf("share not found %w", err) } @@ -991,7 +1055,7 @@ func (d *CSIDriver) ControllerExpandVolume(ctx context.Context, req *csi.Control if shareName == "" { return nil, status.Error(codes.NotFound, common.VolumeNotFound) } - share, err := d.hsclient.GetShare(ctx, shareName) + share, err := hsclient.GetShare(ctx, shareName) if share == nil { return nil, status.Error(codes.NotFound, common.ShareNotFound) } @@ -1003,7 +1067,7 @@ func (d *CSIDriver) ControllerExpandVolume(ctx context.Context, req *csi.Control } if currentSize < requestedSize { - err = d.hsclient.UpdateShareSize(ctx, shareName, requestedSize) + err = hsclient.UpdateShareSize(ctx, shareName, requestedSize) if err != nil { return nil, status.Error(codes.Internal, common.UnknownError) } @@ -1033,13 +1097,30 @@ func (d *CSIDriver) ValidateVolumeCapabilities(ctx context.Context, req *csi.Val return nil, status.Errorf(codes.InvalidArgument, common.NoCapabilitiesSupplied, req.VolumeId) } + hsclient := d.hsclient + if req.Secrets != nil { + if endpoint, ok := req.Secrets["csiEndpoint"]; ok { + username, uOk := req.Secrets["username"] + password, pOk := req.Secrets["password"] + if !uOk || !pOk { + return nil, status.Errorf(codes.InvalidArgument, "username and password must be in secrets") + } + tlsVerify, _ := strconv.ParseBool(req.Secrets["csiTlsVerify"]) + newClient, err := client.NewHammerspaceClient(endpoint, username, password, tlsVerify) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to create new hsclient, %v", err) + } + hsclient = newClient + } + } + // Find Share typeBlock := false typeMount := false fileBacked := false volumeName := GetVolumeNameFromPath(req.GetVolumeId()) - share, _ := d.hsclient.GetShare(ctx, volumeName) + share, _ := hsclient.GetShare(ctx, volumeName) if share != nil { typeMount = true } @@ -1054,7 +1135,7 @@ func (d *CSIDriver) ValidateVolumeCapabilities(ctx context.Context, req *csi.Val // Check if the specified backing share or file exists if share == nil { - backingFileExists, err := d.hsclient.DoesFileExist(ctx, req.GetVolumeId()) + backingFileExists, err := hsclient.DoesFileExist(ctx, req.GetVolumeId()) if err != nil { log.Error(err) } @@ -1111,7 +1192,12 @@ func (d *CSIDriver) ListVolumes(ctx context.Context, req *csi.ListVolumesRequest "[ListVolumes] Invalid max entries request %v, must not be negative ", req.MaxEntries)) } - vlist, err := d.hsclient.ListVolumes(ctx) + hsclient := d.hsclient + // Note: ListVolumes does not have a secrets field in the request. + // We would need to implement a custom way to pass secrets if we want to support this. + // For now, we will use the default client. + + vlist, err := hsclient.ListVolumes(ctx) if err != nil { return nil, status.Error(codes.Internal, fmt.Sprintf("ListVolumes failed: %v", err)) } @@ -1141,6 +1227,23 @@ func (d *CSIDriver) GetCapacity(ctx context.Context, req *csi.GetCapacityRequest ctx, span := tracer.Start(ctx, "Controller/GetCapacity", trace.WithAttributes()) defer span.End() + hsclient := d.hsclient + // if req.Secrets != nil { + // if endpoint, ok := req.Secrets["csiEndpoint"]; ok { + // username, uOk := req.Secrets["username"] + // password, pOk := req.Secrets["password"] + // if !uOk || !pOk { + // return nil, status.Errorf(codes.InvalidArgument, "username and password must be in secrets") + // } + // tlsVerify, _ := strconv.ParseBool(req.Secrets["csiTlsVerify"]) + // newClient, err := client.NewHammerspaceClient(endpoint, username, password, tlsVerify) + // if err != nil { + // return nil, status.Errorf(codes.Internal, "failed to create new hsclient, %v", err) + // } + // hsclient = newClient + // } + // } + var blockRequested bool var filesystemRequested bool fileBacked := false @@ -1179,7 +1282,7 @@ func (d *CSIDriver) GetCapacity(ctx context.Context, req *csi.GetCapacityRequest } else { backingShareName = vParams.MountBackingShareName } - backingShare, err := d.hsclient.GetShare(ctx, backingShareName) + backingShare, err := hsclient.GetShare(ctx, backingShareName) if err != nil { available = 0 } @@ -1189,7 +1292,7 @@ func (d *CSIDriver) GetCapacity(ctx context.Context, req *csi.GetCapacityRequest } else { // Return all capacity of cluster for share backed volumes - available, err = d.hsclient.GetClusterAvailableCapacity(ctx) + available, err = hsclient.GetClusterAvailableCapacity(ctx) if err != nil { return nil, status.Error(codes.Internal, err.Error()) } @@ -1285,22 +1388,39 @@ func (d *CSIDriver) CreateSnapshot(ctx context.Context, req *csi.CreateSnapshotR } defer unlock() + hsclient := d.hsclient + if req.Secrets != nil { + if endpoint, ok := req.Secrets["csiEndpoint"]; ok { + username, uOk := req.Secrets["username"] + password, pOk := req.Secrets["password"] + if !uOk || !pOk { + return nil, status.Errorf(codes.InvalidArgument, "username and password must be in secrets") + } + tlsVerify, _ := strconv.ParseBool(req.Secrets["csiTlsVerify"]) + newClient, err := client.NewHammerspaceClient(endpoint, username, password, tlsVerify) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to create new hsclient, %v", err) + } + hsclient = newClient + } + } + // FIXME: Check to see if snapshot already exists? // (using their id somehow?, update the share extended info maybe?) what about for file-backed volumes? // do we update extended info on backing share? if _, exists := recentlyCreatedSnapshots[req.GetName()]; !exists { // find source volume (is it file or share? volumeName := GetVolumeNameFromPath(req.GetSourceVolumeId()) - share, err := d.hsclient.GetShare(ctx, volumeName) + share, err := hsclient.GetShare(ctx, volumeName) if err != nil { return nil, status.Errorf(codes.Internal, "%s", err.Error()) } // Create the snapshot var hsSnapName string if share != nil { - hsSnapName, err = d.hsclient.SnapshotShare(ctx, volumeName) + hsSnapName, err = hsclient.SnapshotShare(ctx, volumeName) } else { - hsSnapName, err = d.hsclient.SnapshotFile(ctx, req.GetSourceVolumeId()) + hsSnapName, err = hsclient.SnapshotFile(ctx, req.GetSourceVolumeId()) } if err != nil { return nil, status.Errorf(codes.Internal, "%s", err.Error()) @@ -1342,6 +1462,23 @@ func (d *CSIDriver) DeleteSnapshot(ctx context.Context, req *csi.DeleteSnapshotR return nil, status.Error(codes.InvalidArgument, common.EmptySnapshotId) } + hsclient := d.hsclient + if req.Secrets != nil { + if endpoint, ok := req.Secrets["csiEndpoint"]; ok { + username, uOk := req.Secrets["username"] + password, pOk := req.Secrets["password"] + if !uOk || !pOk { + return nil, status.Errorf(codes.InvalidArgument, "username and password must be in secrets") + } + tlsVerify, _ := strconv.ParseBool(req.Secrets["csiTlsVerify"]) + newClient, err := client.NewHammerspaceClient(endpoint, username, password, tlsVerify) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to create new hsclient, %v", err) + } + hsclient = newClient + } + } + splitSnapId := strings.SplitN(snapshotId, "|", 2) if len(splitSnapId) != 2 { log.Warnf("DeleteSnapshot: malformed snapshot ID %s; treating as success (idempotent)", snapshotId) @@ -1355,9 +1492,9 @@ func (d *CSIDriver) DeleteSnapshot(ctx context.Context, req *csi.DeleteSnapshotR var err error if shareName != "" { - err = d.hsclient.DeleteShareSnapshot(ctx, shareName, snapshotName) + err = hsclient.DeleteShareSnapshot(ctx, shareName, snapshotName) } else { - err = d.hsclient.DeleteFileSnapshot(ctx, path, snapshotName) + err = hsclient.DeleteFileSnapshot(ctx, path, snapshotName) } if err != nil { @@ -1386,11 +1523,28 @@ func (d *CSIDriver) ListSnapshots(ctx context.Context, req *csi.ListSnapshotsReq "[ListSnapshots] Invalid max entries request %v, must not be negative ", req.MaxEntries)) } + hsclient := d.hsclient + if req.Secrets != nil { + if endpoint, ok := req.Secrets["csiEndpoint"]; ok { + username, uOk := req.Secrets["username"] + password, pOk := req.Secrets["password"] + if !uOk || !pOk { + return nil, status.Errorf(codes.InvalidArgument, "username and password must be in secrets") + } + tlsVerify, _ := strconv.ParseBool(req.Secrets["csiTlsVerify"]) + newClient, err := client.NewHammerspaceClient(endpoint, username, password, tlsVerify) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to create new hsclient, %v", err) + } + hsclient = newClient + } + } + // Initialize a slice to hold the snapshot entries var snapshots []*csi.ListSnapshotsResponse_Entry // Fetch all snapshots from the backend storage - backendSnapshots, err := d.hsclient.ListSnapshots(ctx, req.SnapshotId, req.SourceVolumeId) + backendSnapshots, err := hsclient.ListSnapshots(ctx, req.SnapshotId, req.SourceVolumeId) if err != nil { return nil, status.Error(codes.Internal, err.Error()) } diff --git a/pkg/driver/node.go b/pkg/driver/node.go index 66e823c..847554e 100644 --- a/pkg/driver/node.go +++ b/pkg/driver/node.go @@ -170,6 +170,11 @@ func (d *CSIDriver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolum return nil, status.Error(codes.InvalidArgument, "VolumeCapability must be provided") } + mountFlags := []string{} + if m := volumeCapability.GetMount(); m != nil { + mountFlags = m.GetMountFlags() + } + log.WithFields(log.Fields{ "volume_id": volumeID, "staging_target": stagingTarget, @@ -190,7 +195,7 @@ func (d *CSIDriver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolum // Step 2: Ensure the root NFS export is mounted once per node // EnsureRootExportMounted function will do a mount check before mounting or creating dir. - if err := d.EnsureRootExportMounted(ctx, common.BaseBackingShareMountPath); err != nil { + if err := d.EnsureRootExportMounted(ctx, common.BaseBackingShareMountPath, mountFlags); err != nil { return nil, status.Errorf(codes.Internal, "root export mount failed: %v", err) } diff --git a/pkg/driver/node_helper.go b/pkg/driver/node_helper.go index f5abe10..ea0da1f 100644 --- a/pkg/driver/node_helper.go +++ b/pkg/driver/node_helper.go @@ -33,7 +33,7 @@ func (d *CSIDriver) publishShareBackedVolume(ctx context.Context, volumeId, targ } // Mount root export (same as NodeStageVolume) - if err := d.EnsureRootExportMounted(ctx, common.BaseBackingShareMountPath); err != nil { + if err := d.EnsureRootExportMounted(ctx, common.BaseBackingShareMountPath, nil); err != nil { return status.Errorf(codes.Internal, "[LazyStage] root export mount failed: %v", err) } diff --git a/pkg/driver/utils.go b/pkg/driver/utils.go index 2309bfe..e626dfa 100644 --- a/pkg/driver/utils.go +++ b/pkg/driver/utils.go @@ -429,7 +429,7 @@ func (d *CSIDriver) MountShareAtBestDataportal(ctx context.Context, shareExportP return fmt.Errorf("could not mount to any data-portals") } -func (d *CSIDriver) EnsureRootExportMounted(ctx context.Context, baseRootDirPath string) error { +func (d *CSIDriver) EnsureRootExportMounted(ctx context.Context, baseRootDirPath string, mountFlags []string) error { log.Debugf("Check if %s is already mounted", baseRootDirPath) if common.IsShareMounted(baseRootDirPath) { log.Debugf("Root dir mount is already mounted at this node on path %s", baseRootDirPath) @@ -446,9 +446,10 @@ func (d *CSIDriver) EnsureRootExportMounted(ctx context.Context, baseRootDirPath } // Step 2 - Use export ip and path to mount root with 4.2 only. log.Debugf("Calling mount via nfs v4.2 using anvil IP %s to mount (/) on %s", "", baseRootDirPath) - var mountOption []string - mountOption = append(mountOption, "nfsvers=4.2") - err = common.MountShare(anvilEndpointIP+":/", baseRootDirPath, mountOption) + if mountFlags == nil { + mountFlags = append(mountFlags, "nfsvers=4.2") + } + err = common.MountShare(anvilEndpointIP+":/", baseRootDirPath, mountFlags) if err != nil { log.Errorf("Unable to mount root share via 4.2 using anvil IP. %v", err) From 576a68bb13ab69718031e8246d3d0dee250b13f1 Mon Sep 17 00:00:00 2001 From: Ravi Kumar Date: Tue, 17 Mar 2026 10:48:05 +0530 Subject: [PATCH 3/4] 1. Added volume handle hash collision mechanish. 2. Used storage class or pv or pvc secret for validation of cred and mount. --- docs/multi-secret-sorageclass.md | 16 ++- docs/static-pv-multi-site.md | 88 ++++++++++++++++ pkg/client/hsclient.go | 18 +++- pkg/driver/controller.go | 169 ++++++++++++------------------- pkg/driver/driver.go | 45 +++++++- pkg/driver/identity.go | 10 +- pkg/driver/node.go | 57 +++++++++-- pkg/driver/node_helper.go | 25 ++--- pkg/driver/utils.go | 23 +++-- 9 files changed, 308 insertions(+), 143 deletions(-) create mode 100644 docs/static-pv-multi-site.md diff --git a/docs/multi-secret-sorageclass.md b/docs/multi-secret-sorageclass.md index c6357cf..46a9fd7 100644 --- a/docs/multi-secret-sorageclass.md +++ b/docs/multi-secret-sorageclass.md @@ -47,6 +47,10 @@ provisioner: csi.hammerspace.com parameters: csi.storage.k8s.io/provisioner-secret-name: hs-secret-1 csi.storage.k8s.io/provisioner-secret-namespace: default + csi.storage.k8s.io/node-stage-secret-name: hs-secret-1 + csi.storage.k8s.io/node-stage-secret-namespace: default + csi.storage.k8s.io/node-publish-secret-name: hs-secret-1 + csi.storage.k8s.io/node-publish-secret-namespace: default # Other parameters specific to your storage provisioner fsType: "nfs" volumeNameFormat: "pvc-%s" @@ -62,6 +66,10 @@ provisioner: csi.hammerspace.com parameters: csi.storage.k8s.io/provisioner-secret-name: hs-secret-2 csi.storage.k8s.io/provisioner-secret-namespace: default + csi.storage.k8s.io/node-stage-secret-name: hs-secret-2 + csi.storage.k8s.io/node-stage-secret-namespace: default + csi.storage.k8s.io/node-publish-secret-name: hs-secret-2 + csi.storage.k8s.io/node-publish-secret-namespace: default # Other parameters specific to your storage provisioner fsType: "xfs" volumeNameFormat: "test-%s" @@ -109,8 +117,14 @@ spec: - Storage Classes: The StorageClass resources define the type of storage to be provisioned. The parameters section includes: csi.storage.k8s.io/provisioner-secret-name: Specifies the name of the secret containing the credentials. csi.storage.k8s.io/provisioner-secret-namespace: Specifies the namespace where the secret is located. + csi.storage.k8s.io/node-stage-secret-name / namespace: Secrets used by NodeStageVolume (root share mount). + csi.storage.k8s.io/node-publish-secret-name / namespace: Secrets used by NodePublishVolume (bind mounts and file/block mounts). Other storage-specific parameters (e.g., fsType, volumeNameFormat). +#### Delete Behavior + +DeleteVolume uses the provisioner secrets when provided. If the delete request does not include secrets, the driver will fall back to the secrets cached at CreateVolume time for the same volume ID. This cache is in-memory and will be lost if the controller restarts, so the recommended setup is to always supply provisioner secrets in the StorageClass. + - PersistentVolumeClaims: The PersistentVolumeClaim resources request storage from the provisioner. The storageClassName field specifies which storage class to use, which in turn determines which secret will be used for provisioning. #### Testing the Configuration @@ -130,4 +144,4 @@ Apply the PVCs: kubectl apply -f hs-pvc-1.yaml kubectl apply -f hs-pvc-2.yaml ``` -After applying these YAML files, the CSI driver should use hs-secret-1 to provision the volume for hs-pvc-secret-1 and hs-secret-2 to provision the volume for hs-pvc-secret-2. You can verify this by inspecting the logs of the CSI driver to see which credentials were used for each provisioning operation. \ No newline at end of file +After applying these YAML files, the CSI driver should use hs-secret-1 to provision the volume for hs-pvc-secret-1 and hs-secret-2 to provision the volume for hs-pvc-secret-2. You can verify this by inspecting the logs of the CSI driver to see which credentials were used for each provisioning operation. diff --git a/docs/static-pv-multi-site.md b/docs/static-pv-multi-site.md new file mode 100644 index 0000000..b1d9823 --- /dev/null +++ b/docs/static-pv-multi-site.md @@ -0,0 +1,88 @@ +# Static PV Multi-Site Mounting (Same Share, Different Clusters) + +This document explains a safe, repeatable workflow for mounting the same Hammerspace share from two sites using **static PVs**, while keeping CSI `volumeHandle` values unique. + +## Why This Matters + +- CSI requires `volumeHandle` (CSI `VolumeId`) to be **globally unique per driver**. +- This driver also uses `volumeHandle` to decide **what path to mount**. +- A mismatch can lead to: + - staging path hash collisions + - lock timeouts + - wrong mount targets + +## Driver Behavior Summary + +When `mountBackingShareName` is set: +- The driver mounts the backing share. +- It then bind-mounts a **subpath inside the share**. +- Default subpath = `volumeHandle` (historical behavior). + +When `mountBackingShareName` is not set: +- The driver treats `volumeHandle` as the **share path itself**. +- There is **no subpath override** in this mode. + +## New Optional Attribute (Static PVs) + +You can now set an explicit subpath using: +- `shareSubPath` (preferred) +- `exportSubPath` or `mountSubPath` (aliases) + +If set, the driver mounts: +`/tmp//` + +If `shareSubPath` is `.` or empty, it mounts the **share root**. + +This is backward compatible. If the attribute is not set, behavior is unchanged. + +## Recommended Workflow (Same Share, Two Sites) + +Use unique `volumeHandle` values, and explicitly mount the share root via `shareSubPath`. + +Example: + +```yaml +apiVersion: v1 +kind: PersistentVolume +metadata: + name: hs-pv-aiml-site1 +spec: + csi: + driver: com.hammerspace.csi + volumeHandle: aiml-site1 + volumeAttributes: + mountBackingShareName: "aiml" + shareSubPath: "." +``` + +```yaml +apiVersion: v1 +kind: PersistentVolume +metadata: + name: hs-pv-aiml-site2 +spec: + csi: + driver: com.hammerspace.csi + volumeHandle: aiml-site2 + volumeAttributes: + mountBackingShareName: "aiml" + shareSubPath: "." +``` + +Both PVs will mount the **same share root**, but the handles remain unique. + +## Common Misconfigurations + +- Using the same `volumeHandle` for both PVs: + - This violates CSI uniqueness and causes staging/lock collisions. +- Setting `mountBackingShareName` but not creating the subpath: + - If you rely on default behavior, `volumeHandle` must exist as a directory inside the share. +- Trying to use subpath overrides without `mountBackingShareName`: + - Not supported in the current driver path. + +## Troubleshooting Checklist + +1. Ensure `volumeHandle` values are unique. +2. If `mountBackingShareName` is set, check the share is mounted at `/tmp/`. +3. If using `shareSubPath`, verify the directory exists inside the share. +4. Look for lock timeout logs tied to duplicate `volumeHandle`. diff --git a/pkg/client/hsclient.go b/pkg/client/hsclient.go index c12f103..f105c55 100755 --- a/pkg/client/hsclient.go +++ b/pkg/client/hsclient.go @@ -101,6 +101,20 @@ func NewHammerspaceClient(endpoint, username, password string, tlsVerify bool) ( return hsclient, err } +func NewHammerspaceClientFromSecrets(secrets map[string]string) (*HammerspaceClient, error) { + endpoint, ok := secrets["csiEndpoint"] + if !ok { + return nil, fmt.Errorf("csiEndpoint must be in secrets") + } + username, uOk := secrets["username"] + password, pOk := secrets["password"] + if !uOk || !pOk { + return nil, fmt.Errorf("username and password must be in secrets") + } + tlsVerify, _ := strconv.ParseBool(secrets["csiTlsVerify"]) + return NewHammerspaceClient(endpoint, username, password, tlsVerify) +} + // GetAnvilPortal returns the hostname of the configured Hammerspace API gateway func (client *HammerspaceClient) GetAnvilPortal() (string, error) { endpointUrl, _ := url.Parse(client.endpoint) @@ -679,7 +693,7 @@ func (client *HammerspaceClient) CreateShare(ctx context.Context, } // Set objectives on share (share-level, not root path) - err = client.SetShareObjectives(ctx, name, objectives, true) + err = client.SetShareObjectives(ctx, name, objectives, false) if err != nil { log.Errorf("Failed to set objectives %s, %v", objectives, err) return err @@ -763,7 +777,7 @@ func (client *HammerspaceClient) CreateShareFromSnapshot(ctx context.Context, na } // Set objectives on share (share-level, not root path) - err = client.SetShareObjectives(ctx, name, objectives, true) + err = client.SetShareObjectives(ctx, name, objectives, false) if err != nil { log.Errorf("Failed to set objectives %s, %v", objectives, err) return err diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index b2138fb..ebe7b1c 100755 --- a/pkg/driver/controller.go +++ b/pkg/driver/controller.go @@ -205,8 +205,8 @@ func (d *CSIDriver) ensureNFSDirectoryExists(ctx context.Context, hsclient *clie deviceFile := targetPath + "/" + hsVolume.Name // mount the share to create the directory - defer d.UnmountBackingShareIfUnused(ctx, backingShare.Name) - err = d.EnsureBackingShareMounted(ctx, backingShare.Name, hsVolume) // check if share is mounted + defer d.UnmountBackingShareIfUnused(ctx, hsclient, backingShare.Name) + err = d.EnsureBackingShareMounted(ctx, hsclient, backingShare.Name, hsVolume) // check if share is mounted if err != nil { log.Errorf("failed to ensure backing share is mounted, %v", err) return err @@ -301,7 +301,7 @@ func (d *CSIDriver) ensureShareBackedVolumeExists(ctx context.Context, hsclient defer common.UnmountFilesystem(targetPath) log.Debugf("Created empty folder with path %s", targetPath) - err = d.publishShareBackedVolume(ctx, hsVolume.Path, targetPath) + err = d.publishShareBackedVolume(ctx, hsclient, hsVolume.Path, targetPath) if err != nil { log.Warnf("failed to get share backed volume on hsVolumePath %s targetPath %s. Err %v", hsVolume.Path, targetPath, err) } @@ -348,7 +348,7 @@ func (d *CSIDriver) ensureBackingShareExists(ctx context.Context, hsclient *clie // generate unique target path on host for setting file metadata targetPath := common.ShareStagingDir + "/metadata-mounts" + hsVolume.Path defer common.UnmountFilesystem(targetPath) - err = d.publishShareBackedVolume(ctx, hsVolume.Path, targetPath) + err = d.publishShareBackedVolume(ctx, hsclient, hsVolume.Path, targetPath) if err != nil { log.Warnf("failed to get share backed volume on hsVolumePath %s targetPath %s. Err %v", hsVolume.Path, targetPath, err) } @@ -408,9 +408,9 @@ func (d *CSIDriver) ensureDeviceFileExists(ctx context.Context, hsclient *client } } else { // Create empty file - defer d.UnmountBackingShareIfUnused(ctx, backingShare.Name) + defer d.UnmountBackingShareIfUnused(ctx, hsclient, backingShare.Name) - err = d.EnsureBackingShareMounted(ctx, backingShare.Name, hsVolume) + err = d.EnsureBackingShareMounted(ctx, hsclient, backingShare.Name, hsVolume) if err != nil { log.Errorf("failed to ensure backing share is mounted, %v", err) return err @@ -549,6 +549,10 @@ func (d *CSIDriver) CreateVolume(ctx context.Context, req *csi.CreateVolumeReque } hsclient := d.hsclient + volumeSecrets := map[string]string{} + for k, v := range req.Secrets { + volumeSecrets[k] = v + } if vParams.SecretName != "" { if vParams.CsiEndpoint == "" { return nil, status.Errorf(codes.InvalidArgument, "csiEndpoint must be specified with secretName") @@ -564,6 +568,10 @@ func (d *CSIDriver) CreateVolume(ctx context.Context, req *csi.CreateVolumeReque } hsclient = newClient } + if vParams.CsiEndpoint != "" { + volumeSecrets["csiEndpoint"] = vParams.CsiEndpoint + volumeSecrets["csiTlsVerify"] = strconv.FormatBool(vParams.CsiTlsVerify) + } // Check for snapshot source specified cs := req.VolumeContentSource @@ -781,6 +789,13 @@ func (d *CSIDriver) CreateVolume(ctx context.Context, req *csi.CreateVolumeReque volContext["mountBackingShareName"] = hsVolume.MountBackingShareName volContext["fsType"] = fsType } + if vParams.SecretName != "" { + volContext["csiSecretName"] = vParams.SecretName + volContext["csiSecretNamespace"] = vParams.SecretNamespace + } + if endpoint := volumeSecrets["csiEndpoint"]; endpoint != "" { + volContext["csiEndpoint"] = endpoint + } log.Infof("Total time taken for create volume %v", time.Since(startTime)) @@ -803,6 +818,9 @@ func (d *CSIDriver) CreateVolume(ctx context.Context, req *csi.CreateVolumeReque } log.WithField("response", resp).Info("volume was created") + if volumeSecrets["csiEndpoint"] != "" { + d.storeVolumeSecrets(volID, volumeSecrets) + } return resp, nil } @@ -837,8 +855,8 @@ func (d *CSIDriver) deleteFileBackedVolume(ctx context.Context, hsclient *client } defer unlock() // mount the share to delete the file - defer d.UnmountBackingShareIfUnused(ctx, residingShareName) - err = d.EnsureBackingShareMounted(ctx, residingShareName, hsVolume) // check if share is mounted + defer d.UnmountBackingShareIfUnused(ctx, hsclient, residingShareName) + err = d.EnsureBackingShareMounted(ctx, hsclient, residingShareName, hsVolume) // check if share is mounted if err != nil { log.Errorf("failed to ensure backing share is mounted, %v", err) return status.Errorf(codes.Internal, "%s", err.Error()) @@ -900,21 +918,9 @@ func (d *CSIDriver) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeReque } defer unlock() - hsclient := d.hsclient - if req.Secrets != nil { - if endpoint, ok := req.Secrets["csiEndpoint"]; ok { - username, uOk := req.Secrets["username"] - password, pOk := req.Secrets["password"] - if !uOk || !pOk { - return nil, status.Errorf(codes.InvalidArgument, "username and password must be in secrets") - } - tlsVerify, _ := strconv.ParseBool(req.Secrets["csiTlsVerify"]) - newClient, err := client.NewHammerspaceClient(endpoint, username, password, tlsVerify) - if err != nil { - return nil, status.Errorf(codes.Internal, "failed to create new hsclient, %v", err) - } - hsclient = newClient - } + hsclient, err := d.clientFromSecretsOrVolume(req.Secrets, volumeId) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to create hsclient from secrets, %v", err) } volumeName := GetVolumeNameFromPath(volumeId) @@ -924,10 +930,15 @@ func (d *CSIDriver) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeReque } if share == nil { // Share does not exist, may be a file-backed volume err = d.deleteFileBackedVolume(ctx, hsclient, volumeId) - + if err == nil { + d.deleteVolumeSecrets(volumeId) + } return &csi.DeleteVolumeResponse{}, err } else { // Share exists and is a Filesystem err = d.deleteShareBackedVolume(ctx, hsclient, share) + if err == nil { + d.deleteVolumeSecrets(volumeId) + } return &csi.DeleteVolumeResponse{}, err } @@ -973,31 +984,24 @@ func (d *CSIDriver) ControllerExpandVolume(ctx context.Context, req *csi.Control return nil, status.Error(codes.InvalidArgument, common.VolumeNotFound) } + var err error hsclient := d.hsclient if req.Secrets != nil { - if endpoint, ok := req.Secrets["csiEndpoint"]; ok { - username, uOk := req.Secrets["username"] - password, pOk := req.Secrets["password"] - if !uOk || !pOk { - return nil, status.Errorf(codes.InvalidArgument, "username and password must be in secrets") - } - tlsVerify, _ := strconv.ParseBool(req.Secrets["csiTlsVerify"]) - newClient, err := client.NewHammerspaceClient(endpoint, username, password, tlsVerify) - if err != nil { - return nil, status.Errorf(codes.Internal, "failed to create new hsclient, %v", err) - } - hsclient = newClient + hsclient, err = client.NewHammerspaceClientFromSecrets(req.Secrets) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to create hsclient from secrets, %v", err) } } volumeName := GetVolumeNameFromPath(req.GetVolumeId()) - share, _ := hsclient.GetShare(ctx, volumeName) - if share == nil { - fileBacked = true + share, err := hsclient.GetShare(ctx, volumeName) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to get share before expand: %s", err.Error()) } // Check if the specified backing share or file exists if share == nil { + fileBacked = true backingFileExists, err := hsclient.DoesFileExist(ctx, req.GetVolumeId()) if err != nil { log.Error(err) @@ -1097,20 +1101,12 @@ func (d *CSIDriver) ValidateVolumeCapabilities(ctx context.Context, req *csi.Val return nil, status.Errorf(codes.InvalidArgument, common.NoCapabilitiesSupplied, req.VolumeId) } + var err error hsclient := d.hsclient if req.Secrets != nil { - if endpoint, ok := req.Secrets["csiEndpoint"]; ok { - username, uOk := req.Secrets["username"] - password, pOk := req.Secrets["password"] - if !uOk || !pOk { - return nil, status.Errorf(codes.InvalidArgument, "username and password must be in secrets") - } - tlsVerify, _ := strconv.ParseBool(req.Secrets["csiTlsVerify"]) - newClient, err := client.NewHammerspaceClient(endpoint, username, password, tlsVerify) - if err != nil { - return nil, status.Errorf(codes.Internal, "failed to create new hsclient, %v", err) - } - hsclient = newClient + hsclient, err = client.NewHammerspaceClientFromSecrets(req.Secrets) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to create hsclient from secrets, %v", err) } } @@ -1227,22 +1223,9 @@ func (d *CSIDriver) GetCapacity(ctx context.Context, req *csi.GetCapacityRequest ctx, span := tracer.Start(ctx, "Controller/GetCapacity", trace.WithAttributes()) defer span.End() - hsclient := d.hsclient - // if req.Secrets != nil { - // if endpoint, ok := req.Secrets["csiEndpoint"]; ok { - // username, uOk := req.Secrets["username"] - // password, pOk := req.Secrets["password"] - // if !uOk || !pOk { - // return nil, status.Errorf(codes.InvalidArgument, "username and password must be in secrets") - // } - // tlsVerify, _ := strconv.ParseBool(req.Secrets["csiTlsVerify"]) - // newClient, err := client.NewHammerspaceClient(endpoint, username, password, tlsVerify) - // if err != nil { - // return nil, status.Errorf(codes.Internal, "failed to create new hsclient, %v", err) - // } - // hsclient = newClient - // } - // } + // Note: GetCapacity does not have a secrets field in the request. + // We would need to implement a custom way to pass secrets if we want to support this. + // For now, we will use the default client. var blockRequested bool var filesystemRequested bool @@ -1282,7 +1265,7 @@ func (d *CSIDriver) GetCapacity(ctx context.Context, req *csi.GetCapacityRequest } else { backingShareName = vParams.MountBackingShareName } - backingShare, err := hsclient.GetShare(ctx, backingShareName) + backingShare, err := d.hsclient.GetShare(ctx, backingShareName) if err != nil { available = 0 } @@ -1292,7 +1275,7 @@ func (d *CSIDriver) GetCapacity(ctx context.Context, req *csi.GetCapacityRequest } else { // Return all capacity of cluster for share backed volumes - available, err = hsclient.GetClusterAvailableCapacity(ctx) + available, err = d.hsclient.GetClusterAvailableCapacity(ctx) if err != nil { return nil, status.Error(codes.Internal, err.Error()) } @@ -1390,18 +1373,9 @@ func (d *CSIDriver) CreateSnapshot(ctx context.Context, req *csi.CreateSnapshotR hsclient := d.hsclient if req.Secrets != nil { - if endpoint, ok := req.Secrets["csiEndpoint"]; ok { - username, uOk := req.Secrets["username"] - password, pOk := req.Secrets["password"] - if !uOk || !pOk { - return nil, status.Errorf(codes.InvalidArgument, "username and password must be in secrets") - } - tlsVerify, _ := strconv.ParseBool(req.Secrets["csiTlsVerify"]) - newClient, err := client.NewHammerspaceClient(endpoint, username, password, tlsVerify) - if err != nil { - return nil, status.Errorf(codes.Internal, "failed to create new hsclient, %v", err) - } - hsclient = newClient + hsclient, err = client.NewHammerspaceClientFromSecrets(req.Secrets) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to create hsclient from secrets, %v", err) } } @@ -1462,20 +1436,12 @@ func (d *CSIDriver) DeleteSnapshot(ctx context.Context, req *csi.DeleteSnapshotR return nil, status.Error(codes.InvalidArgument, common.EmptySnapshotId) } + var err error hsclient := d.hsclient if req.Secrets != nil { - if endpoint, ok := req.Secrets["csiEndpoint"]; ok { - username, uOk := req.Secrets["username"] - password, pOk := req.Secrets["password"] - if !uOk || !pOk { - return nil, status.Errorf(codes.InvalidArgument, "username and password must be in secrets") - } - tlsVerify, _ := strconv.ParseBool(req.Secrets["csiTlsVerify"]) - newClient, err := client.NewHammerspaceClient(endpoint, username, password, tlsVerify) - if err != nil { - return nil, status.Errorf(codes.Internal, "failed to create new hsclient, %v", err) - } - hsclient = newClient + hsclient, err = client.NewHammerspaceClientFromSecrets(req.Secrets) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to create hsclient from secrets, %v", err) } } @@ -1490,7 +1456,6 @@ func (d *CSIDriver) DeleteSnapshot(ctx context.Context, req *csi.DeleteSnapshotR shareName := GetVolumeNameFromPath(path) - var err error if shareName != "" { err = hsclient.DeleteShareSnapshot(ctx, shareName, snapshotName) } else { @@ -1523,20 +1488,12 @@ func (d *CSIDriver) ListSnapshots(ctx context.Context, req *csi.ListSnapshotsReq "[ListSnapshots] Invalid max entries request %v, must not be negative ", req.MaxEntries)) } + var err error hsclient := d.hsclient if req.Secrets != nil { - if endpoint, ok := req.Secrets["csiEndpoint"]; ok { - username, uOk := req.Secrets["username"] - password, pOk := req.Secrets["password"] - if !uOk || !pOk { - return nil, status.Errorf(codes.InvalidArgument, "username and password must be in secrets") - } - tlsVerify, _ := strconv.ParseBool(req.Secrets["csiTlsVerify"]) - newClient, err := client.NewHammerspaceClient(endpoint, username, password, tlsVerify) - if err != nil { - return nil, status.Errorf(codes.Internal, "failed to create new hsclient, %v", err) - } - hsclient = newClient + hsclient, err = client.NewHammerspaceClientFromSecrets(req.Secrets) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to create hsclient from secrets, %v", err) } } diff --git a/pkg/driver/driver.go b/pkg/driver/driver.go index 8238cd3..90ca4c1 100644 --- a/pkg/driver/driver.go +++ b/pkg/driver/driver.go @@ -52,6 +52,11 @@ type CSIDriver struct { snapshotLocks map[string]*keyLock hsclient *client.HammerspaceClient NodeID string + secretsMu sync.RWMutex + volumeSecrets map[string]map[string]string + nodeInfoMu sync.RWMutex + nodeInfoKnown bool + nodeIsPortal bool } func NewCSIDriver(endpoint, username, password, tlsVerifyStr string) *CSIDriver { @@ -74,6 +79,7 @@ func NewCSIDriver(endpoint, username, password, tlsVerifyStr string) *CSIDriver volumeLocks: make(map[string]*keyLock), snapshotLocks: make(map[string]*keyLock), NodeID: os.Getenv("CSI_NODE_NAME"), + volumeSecrets: make(map[string]map[string]string), } } @@ -109,7 +115,7 @@ func (c *CSIDriver) acquireVolumeLock(ctx context.Context, volID string) (func() defer cancel() if err := lk.lock(lctx); err != nil { - log.WithError(err).Errorf("Error acquiring volume lock for %s", volID) + log.WithError(err).Errorf("Error acquiring volume lock for %s; exiting to allow supervisor restart", volID) debug.PrintStack() os.Exit(1) } @@ -137,6 +143,43 @@ func (c *CSIDriver) acquireSnapshotLock(ctx context.Context, snapID string) (fun return func() { lk.unlock() }, nil } +func (d *CSIDriver) storeVolumeSecrets(volumeID string, secrets map[string]string) { + if volumeID == "" || len(secrets) == 0 { + return + } + dup := make(map[string]string, len(secrets)) + for k, v := range secrets { + dup[k] = v + } + d.secretsMu.Lock() + d.volumeSecrets[volumeID] = dup + d.secretsMu.Unlock() +} + +func (d *CSIDriver) deleteVolumeSecrets(volumeID string) { + if volumeID == "" { + return + } + d.secretsMu.Lock() + delete(d.volumeSecrets, volumeID) + d.secretsMu.Unlock() +} + +func (d *CSIDriver) clientFromSecretsOrVolume(secrets map[string]string, volumeID string) (*client.HammerspaceClient, error) { + if len(secrets) > 0 { + return client.NewHammerspaceClientFromSecrets(secrets) + } + if volumeID != "" { + d.secretsMu.RLock() + cached, ok := d.volumeSecrets[volumeID] + d.secretsMu.RUnlock() + if ok && len(cached) > 0 { + return client.NewHammerspaceClientFromSecrets(cached) + } + } + return d.hsclient, nil +} + func (c *CSIDriver) goServe(started chan<- bool) { c.wg.Add(1) go func() { diff --git a/pkg/driver/identity.go b/pkg/driver/identity.go index ac7cf03..babd8ff 100644 --- a/pkg/driver/identity.go +++ b/pkg/driver/identity.go @@ -21,8 +21,7 @@ import ( "github.com/container-storage-interface/spec/lib/go/csi" "github.com/hammer-space/csi-plugin/pkg/common" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" + log "github.com/sirupsen/logrus" wrappers "google.golang.org/protobuf/types/known/wrapperspb" ) @@ -49,9 +48,12 @@ func (d *CSIDriver) Probe( // Make sure the client and backend can communicate err := d.hsclient.EnsureLogin() if err != nil { + log.WithError(err).Warn("Probe backend login failed; reporting Ready=true for tolerance") + // Tolerant probe: log and report ready to avoid flapping on transient backend issues. + // This keeps node service available for mounts that use per-request creds. return &csi.ProbeResponse{ - Ready: &wrappers.BoolValue{Value: false}, - }, status.Errorf(codes.Unavailable, "%s", err.Error()) + Ready: &wrappers.BoolValue{Value: true}, + }, nil } return &csi.ProbeResponse{ diff --git a/pkg/driver/node.go b/pkg/driver/node.go index 847554e..2d828f4 100644 --- a/pkg/driver/node.go +++ b/pkg/driver/node.go @@ -18,14 +18,17 @@ package driver import ( "os" + "path" "path/filepath" "strconv" + "strings" "syscall" "unsafe" "context" "github.com/container-storage-interface/spec/lib/go/csi" + "github.com/hammer-space/csi-plugin/pkg/client" "github.com/hammer-space/csi-plugin/pkg/common" log "github.com/sirupsen/logrus" "google.golang.org/grpc/codes" @@ -170,6 +173,11 @@ func (d *CSIDriver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolum return nil, status.Error(codes.InvalidArgument, "VolumeCapability must be provided") } + hsclient, err := d.clientFromSecrets(req.GetSecrets()) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to create hsclient from secrets, %v", err) + } + mountFlags := []string{} if m := volumeCapability.GetMount(); m != nil { mountFlags = m.GetMountFlags() @@ -188,14 +196,14 @@ func (d *CSIDriver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolum marker := GetHashedMarkerPath(common.BaseVolumeMarkerSourcePath, volumeID) - err := os.WriteFile(marker, []byte(""), 0644) + err = os.WriteFile(marker, []byte(""), 0644) if err != nil { log.Warnf("Not able to create marker file path %s err %v", marker, err) } // Step 2: Ensure the root NFS export is mounted once per node // EnsureRootExportMounted function will do a mount check before mounting or creating dir. - if err := d.EnsureRootExportMounted(ctx, common.BaseBackingShareMountPath, mountFlags); err != nil { + if err := d.EnsureRootExportMounted(ctx, hsclient, common.BaseBackingShareMountPath, mountFlags); err != nil { return nil, status.Errorf(codes.Internal, "root export mount failed: %v", err) } @@ -261,6 +269,11 @@ func (d *CSIDriver) NodePublishVolume(ctx context.Context, req *csi.NodePublishV } } + hsclient, err := d.clientFromSecrets(req.GetSecrets()) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to create hsclient from secrets, %v", err) + } + unlock, err := d.acquireVolumeLock(ctx, volume_id) if err != nil { log.Errorf("Failed to acquire volume lock for volume %s: %v", volume_id, err) @@ -300,7 +313,7 @@ func (d *CSIDriver) NodePublishVolume(ctx context.Context, req *csi.NodePublishV "Volume_id": volume_id, "Traget Path": targetPath, }).Info("Starting node publish volume for Share backed NFS volume without backing share.") - err := d.publishShareBackedVolume(ctx, volume_id, targetPath) + err := d.publishShareBackedVolume(ctx, hsclient, volume_id, targetPath) if err != nil { return nil, err } @@ -310,7 +323,32 @@ func (d *CSIDriver) NodePublishVolume(ctx context.Context, req *csi.NodePublishV "Volume_id": volume_id, "Traget Path": targetPath, }).Info("Starting node publish volume for Share backed NFS volume with backing share.") - err := d.publishShareBackedDirBasedVolume(ctx, backingShareName, volume_id, targetPath, fsType, mountFlags, volumeContext["fqdn"]) + exportPath := volume_id + subPath := volumeContext["shareSubPath"] + if subPath == "" { + subPath = volumeContext["exportSubPath"] + } + if subPath == "" { + subPath = volumeContext["mountSubPath"] + } + if subPath != "" { + base := "/" + strings.Trim(backingShareName, "/") + sp := strings.TrimPrefix(subPath, "/") + if sp == "" || sp == "." { + exportPath = base + } else { + exportPath = path.Join(base, sp) + } + if !strings.HasPrefix(exportPath, "/") { + exportPath = "/" + exportPath + } + log.WithFields(log.Fields{ + "backingShareName": backingShareName, + "exportPath": exportPath, + "subPath": subPath, + }).Debug("Using explicit subpath inside backing share for mount.") + } + err := d.publishShareBackedDirBasedVolume(ctx, hsclient, backingShareName, exportPath, targetPath, fsType, mountFlags, volumeContext["fqdn"]) if err != nil { return nil, err } @@ -320,7 +358,7 @@ func (d *CSIDriver) NodePublishVolume(ctx context.Context, req *csi.NodePublishV "Volume_id": volume_id, "Traget Path": targetPath, }).Info("Starting node publish volume file backed.") - err := d.publishFileBackedVolume(ctx, backingShareName, volume_id, targetPath, fsType, mountFlags, readOnly, volumeContext["fqdn"]) + err := d.publishFileBackedVolume(ctx, hsclient, backingShareName, volume_id, targetPath, fsType, mountFlags, readOnly, volumeContext["fqdn"]) if err != nil { log.Errorf("Error while running publishFileBackedVolume.") return nil, err @@ -382,7 +420,7 @@ func (d *CSIDriver) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpubl switch mode := fi.Mode(); { case IsBlockDevice(fi): // block device log.Infof("Detected block device at target path %s", targetPath) - if err := d.unpublishFileBackedVolume(ctx, req.GetVolumeId(), targetPath); err != nil { + if err := d.unpublishFileBackedVolume(ctx, d.hsclient, req.GetVolumeId(), targetPath); err != nil { return nil, err } case mode.IsDir(): // directory for mount volumes @@ -401,6 +439,13 @@ func (d *CSIDriver) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpubl return &csi.NodeUnpublishVolumeResponse{}, nil } +func (d *CSIDriver) clientFromSecrets(secrets map[string]string) (*client.HammerspaceClient, error) { + if len(secrets) == 0 { + return d.hsclient, nil + } + return client.NewHammerspaceClientFromSecrets(secrets) +} + func (d *CSIDriver) NodeGetCapabilities(ctx context.Context, req *csi.NodeGetCapabilitiesRequest) (*csi.NodeGetCapabilitiesResponse, error) { return &csi.NodeGetCapabilitiesResponse{ diff --git a/pkg/driver/node_helper.go b/pkg/driver/node_helper.go index ea0da1f..4174e5b 100644 --- a/pkg/driver/node_helper.go +++ b/pkg/driver/node_helper.go @@ -9,6 +9,7 @@ import ( "context" + "github.com/hammer-space/csi-plugin/pkg/client" "github.com/hammer-space/csi-plugin/pkg/common" log "github.com/sirupsen/logrus" "google.golang.org/grpc/codes" @@ -16,7 +17,7 @@ import ( ) // Mount share and attach it -func (d *CSIDriver) publishShareBackedVolume(ctx context.Context, volumeId, targetPath string) error { +func (d *CSIDriver) publishShareBackedVolume(ctx context.Context, hsclient *client.HammerspaceClient, volumeId, targetPath string) error { // Step 0 — Ensure root share mount exists for this volume (lazy stage for old volumes) // Lazy stage for old volumes (skip if root share already mounted) rootShareMounted, _ := common.SafeIsMountPoint(common.BaseBackingShareMountPath) @@ -33,7 +34,7 @@ func (d *CSIDriver) publishShareBackedVolume(ctx context.Context, volumeId, targ } // Mount root export (same as NodeStageVolume) - if err := d.EnsureRootExportMounted(ctx, common.BaseBackingShareMountPath, nil); err != nil { + if err := d.EnsureRootExportMounted(ctx, hsclient, common.BaseBackingShareMountPath, nil); err != nil { return status.Errorf(codes.Internal, "[LazyStage] root export mount failed: %v", err) } @@ -118,7 +119,7 @@ func (d *CSIDriver) publishShareBackedVolume(ctx context.Context, volumeId, targ } // Check base pv exist as backingShareName and create path with backingShareName/exportPath attach to target path -func (d *CSIDriver) publishShareBackedDirBasedVolume(ctx context.Context, backingShareName, exportPath, targetPath, fsType string, mountFlags []string, fqdn string) error { +func (d *CSIDriver) publishShareBackedDirBasedVolume(ctx context.Context, hsclient *client.HammerspaceClient, backingShareName, exportPath, targetPath, fsType string, mountFlags []string, fqdn string) error { log.Debugf("Recived publish dir based volume request.") unlock, err := d.acquireVolumeLock(ctx, backingShareName) if err != nil { @@ -153,7 +154,7 @@ func (d *CSIDriver) publishShareBackedDirBasedVolume(ctx context.Context, backin log.Infof("check nfs backed volume %v", hsVolume) // Ensure the backing share is mounted - if err := d.EnsureBackingShareMounted(ctx, backingShareName, hsVolume); err != nil { + if err := d.EnsureBackingShareMounted(ctx, hsclient, backingShareName, hsVolume); err != nil { return err } @@ -174,7 +175,7 @@ func (d *CSIDriver) publishShareBackedDirBasedVolume(ctx context.Context, backin if err := common.BindMountDevice(sourceMountPoint, targetPath); err != nil { log.Errorf("bind mount failed for %s: %v", targetPath, err) CleanupLoopDevice(targetPath) - d.UnmountBackingShareIfUnused(ctx, backingShareName) + d.UnmountBackingShareIfUnused(ctx, hsclient, backingShareName) return err } @@ -182,7 +183,7 @@ func (d *CSIDriver) publishShareBackedDirBasedVolume(ctx context.Context, backin return nil } -func (d *CSIDriver) publishFileBackedVolume(ctx context.Context, backingShareName, volumePath, targetPath, fsType string, mountFlags []string, readOnly bool, fqdn string) error { +func (d *CSIDriver) publishFileBackedVolume(ctx context.Context, hsclient *client.HammerspaceClient, backingShareName, volumePath, targetPath, fsType string, mountFlags []string, readOnly bool, fqdn string) error { unlock, err := d.acquireVolumeLock(ctx, backingShareName) if err != nil { // surfaces to kubelet instead of hanging forever @@ -238,7 +239,7 @@ func (d *CSIDriver) publishFileBackedVolume(ctx context.Context, backingShareNam }).Info("Publish file backed volume.") // Ensure the backing share is mounted - if err := d.EnsureBackingShareMounted(ctx, backingShareName, hsVolume); err != nil { + if err := d.EnsureBackingShareMounted(ctx, hsclient, backingShareName, hsVolume); err != nil { return err } @@ -251,7 +252,7 @@ func (d *CSIDriver) publishFileBackedVolume(ctx context.Context, backingShareNam if err != nil { log.Errorf("failed to attach loop device: %v", err) CleanupLoopDevice(deviceStr) - d.UnmountBackingShareIfUnused(ctx, backingShareName) + d.UnmountBackingShareIfUnused(ctx, hsclient, backingShareName) return status.Errorf(codes.Internal, common.LoopDeviceAttachFailed, deviceStr, filePath) } log.Infof("File %s attached to %s", filePath, deviceStr) @@ -259,7 +260,7 @@ func (d *CSIDriver) publishFileBackedVolume(ctx context.Context, backingShareNam if err := common.BindMountDevice(deviceStr, targetPath); err != nil { log.Errorf("bind mount failed for %s: %v", deviceStr, err) CleanupLoopDevice(deviceStr) - d.UnmountBackingShareIfUnused(ctx, backingShareName) + d.UnmountBackingShareIfUnused(ctx, hsclient, backingShareName) return err } } else { @@ -267,7 +268,7 @@ func (d *CSIDriver) publishFileBackedVolume(ctx context.Context, backingShareNam mountFlags = append(mountFlags, "ro") } if err := common.MountFilesystem(filePath, targetPath, fsType, mountFlags); err != nil { - d.UnmountBackingShareIfUnused(ctx, backingShareName) + d.UnmountBackingShareIfUnused(ctx, hsclient, backingShareName) return err } } @@ -275,7 +276,7 @@ func (d *CSIDriver) publishFileBackedVolume(ctx context.Context, backingShareNam } // NodeUnpublishVolume -func (d *CSIDriver) unpublishFileBackedVolume(ctx context.Context, volumePath, targetPath string) error { +func (d *CSIDriver) unpublishFileBackedVolume(ctx context.Context, hsclient *client.HammerspaceClient, volumePath, targetPath string) error { //determine backing share backingShareName := filepath.Dir(volumePath) @@ -318,7 +319,7 @@ func (d *CSIDriver) unpublishFileBackedVolume(ctx context.Context, volumePath, t } // Unmount backing share if appropriate - unmounted, err := d.UnmountBackingShareIfUnused(ctx, backingShareName) + unmounted, err := d.UnmountBackingShareIfUnused(ctx, hsclient, backingShareName) if unmounted { log.Infof("unmounted backing share, %s", backingShareName) } diff --git a/pkg/driver/utils.go b/pkg/driver/utils.go index e626dfa..eda25cf 100644 --- a/pkg/driver/utils.go +++ b/pkg/driver/utils.go @@ -29,6 +29,7 @@ import ( "context" + "github.com/hammer-space/csi-plugin/pkg/client" log "github.com/sirupsen/logrus" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -207,8 +208,8 @@ func GetSnapshotIDFromSnapshotName(hsSnapName, sourceVolumeID string) string { return fmt.Sprintf("%s|%s", hsSnapName, sourceVolumeID) } -func (d *CSIDriver) EnsureBackingShareMounted(ctx context.Context, backingShareName string, hsVol *common.HSVolume) error { - backingShare, err := d.hsclient.GetShare(ctx, backingShareName) +func (d *CSIDriver) EnsureBackingShareMounted(ctx context.Context, hsclient *client.HammerspaceClient, backingShareName string, hsVol *common.HSVolume) error { + backingShare, err := hsclient.GetShare(ctx, backingShareName) if err != nil { return status.Errorf(codes.NotFound, "%s", err.Error()) } @@ -218,7 +219,7 @@ func (d *CSIDriver) EnsureBackingShareMounted(ctx context.Context, backingShareN isMounted := common.IsShareMounted(backingDir) log.Infof("Checked mount for %s: isMounted=%t", backingDir, isMounted) if !isMounted { - err := d.MountShareAtBestDataportal(ctx, backingShare.ExportPath, backingDir, hsVol.ClientMountOptions, hsVol.FQDN) + err := d.MountShareAtBestDataportal(ctx, hsclient, backingShare.ExportPath, backingDir, hsVol.ClientMountOptions, hsVol.FQDN) if err != nil { log.Errorf("failed to mount backing share, %v", err) return err @@ -233,9 +234,9 @@ func (d *CSIDriver) EnsureBackingShareMounted(ctx context.Context, backingShareN return nil } -func (d *CSIDriver) UnmountBackingShareIfUnused(ctx context.Context, backingShareName string) (bool, error) { +func (d *CSIDriver) UnmountBackingShareIfUnused(ctx context.Context, hsclient *client.HammerspaceClient, backingShareName string) (bool, error) { log.Infof("UnmountBackingShareIfUnused is called with backing share name %s", backingShareName) - backingShare, err := d.hsclient.GetShare(ctx, backingShareName) + backingShare, err := hsclient.GetShare(ctx, backingShareName) if err != nil || backingShare == nil { log.Errorf("unable to get share while checking UnmountBackingShareIfUnused. Err %v", err) return false, err @@ -278,13 +279,13 @@ func (d *CSIDriver) UnmountBackingShareIfUnused(ctx context.Context, backingShar // If we have the IP's in list we use that IP only. We select the IP which response first rpcinfo command. // 3. If all above check is null of err use anvil IP. -func (d *CSIDriver) MountShareAtBestDataportal(ctx context.Context, shareExportPath, targetPath string, mountFlags []string, fqdn string) error { +func (d *CSIDriver) MountShareAtBestDataportal(ctx context.Context, hsclient *client.HammerspaceClient, shareExportPath, targetPath string, mountFlags []string, fqdn string) error { var err error var fipaddr string = "" log.Infof("Finding best host exporting %s", shareExportPath) - portals, err := d.hsclient.GetDataPortals(ctx, d.NodeID) + portals, err := hsclient.GetDataPortals(ctx, d.NodeID) if err != nil { log.WithFields(log.Fields{ "share": shareExportPath, @@ -309,7 +310,7 @@ func (d *CSIDriver) MountShareAtBestDataportal(ctx context.Context, shareExportP } } else { // Always look for floating data portal IPs - fipaddr, err = d.hsclient.GetPortalFloatingIp(ctx) + fipaddr, err = hsclient.GetPortalFloatingIp(ctx) if err != nil { log.Errorf("Could not contact Anvil for floating IPs, %v", err) } @@ -429,7 +430,7 @@ func (d *CSIDriver) MountShareAtBestDataportal(ctx context.Context, shareExportP return fmt.Errorf("could not mount to any data-portals") } -func (d *CSIDriver) EnsureRootExportMounted(ctx context.Context, baseRootDirPath string, mountFlags []string) error { +func (d *CSIDriver) EnsureRootExportMounted(ctx context.Context, hsclient *client.HammerspaceClient, baseRootDirPath string, mountFlags []string) error { log.Debugf("Check if %s is already mounted", baseRootDirPath) if common.IsShareMounted(baseRootDirPath) { log.Debugf("Root dir mount is already mounted at this node on path %s", baseRootDirPath) @@ -440,7 +441,7 @@ func (d *CSIDriver) EnsureRootExportMounted(ctx context.Context, baseRootDirPath return err } // Step 1 - Get Anvil IP - anvilEndpointIP, err := d.hsclient.GetAnvilPortal() + anvilEndpointIP, err := hsclient.GetAnvilPortal() if err != nil { log.Errorf("Not able to extract anvil endpoint. Err %v", err) } @@ -455,7 +456,7 @@ func (d *CSIDriver) EnsureRootExportMounted(ctx context.Context, baseRootDirPath // Step 3 - Use fallback log.Debugf("Call for mount root share with anvil IP and 4.2 FAILED, now will do a fallback try with other data portals, with fallback to 4.2 and v3") - err = d.MountShareAtBestDataportal(ctx, "/", baseRootDirPath, nil, "") + err = d.MountShareAtBestDataportal(ctx, hsclient, "/", baseRootDirPath, nil, "") if err != nil { log.Errorf("Not able to mount root share to mount point %s. Error %v", baseRootDirPath, err) return err From e85be08ec36b0a3412c5d55e13941b10cd26ec55 Mon Sep 17 00:00:00 2001 From: Ravi Kumar Date: Thu, 26 Mar 2026 18:49:59 +0530 Subject: [PATCH 4/4] Removed storing of secret so security purpose. --- CHANGELOG.md | 6 + deploy/kubernetes/example_secret.yaml | 2 + .../kubernetes-1.10-1.12/example_secret.yaml | 2 + deploy/kubernetes/kubernetes-1.29/plugin.yaml | 364 ++++++++++++++++++ docs/multi-secret-sorageclass.md | 10 +- pkg/driver/controller.go | 11 +- pkg/driver/driver.go | 40 -- 7 files changed, 383 insertions(+), 52 deletions(-) create mode 100644 deploy/kubernetes/kubernetes-1.29/plugin.yaml diff --git a/CHANGELOG.md b/CHANGELOG.md index cc42296..7f93b73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,10 +6,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- Multi-tenant / multi-endpoint secret support via StorageClass `csi.storage.k8s.io/*-secret-*` parameters for controller and node operations (provision, stage, publish, delete). + ### Fixed - Passed mount options from the storage class to the mount command. - Fixed an issue where objectives was being applied to share "/" instead it should directly apply to share. +- Ensured controller operations (including `DeleteVolume`) use request-provided secrets when present, to avoid cross-tenant deletes in multi-endpoint environments. +### Documentation +- Clarified that Kubernetes Secrets are Hammerspace management/API (REST) credentials (not NFS mount credentials) and documented DeleteVolume behavior for multi-tenant setups. ## [1.2.8] ### Added diff --git a/deploy/kubernetes/example_secret.yaml b/deploy/kubernetes/example_secret.yaml index 5608d11..9d14593 100644 --- a/deploy/kubernetes/example_secret.yaml +++ b/deploy/kubernetes/example_secret.yaml @@ -4,6 +4,8 @@ metadata: name: com.hammerspace.csi.credentials namespace: kube-system type: Opaque +# This Secret provides the CSI driver's Hammerspace management/API (REST) credentials. +# It is not used as an NFS mount credential. data: username: YWRtaW4= password: YWRtaW4= diff --git a/deploy/kubernetes/kubernetes-1.10-1.12/example_secret.yaml b/deploy/kubernetes/kubernetes-1.10-1.12/example_secret.yaml index 5608d11..9d14593 100644 --- a/deploy/kubernetes/kubernetes-1.10-1.12/example_secret.yaml +++ b/deploy/kubernetes/kubernetes-1.10-1.12/example_secret.yaml @@ -4,6 +4,8 @@ metadata: name: com.hammerspace.csi.credentials namespace: kube-system type: Opaque +# This Secret provides the CSI driver's Hammerspace management/API (REST) credentials. +# It is not used as an NFS mount credential. data: username: YWRtaW4= password: YWRtaW4= diff --git a/deploy/kubernetes/kubernetes-1.29/plugin.yaml b/deploy/kubernetes/kubernetes-1.29/plugin.yaml new file mode 100644 index 0000000..9a779bb --- /dev/null +++ b/deploy/kubernetes/kubernetes-1.29/plugin.yaml @@ -0,0 +1,364 @@ +#### CSI Object +apiVersion: storage.k8s.io/v1 +kind: CSIDriver +metadata: + name: com.hammerspace.csi +spec: + podInfoOnMount: true + requiresRepublish: true + volumeLifecycleModes: + - Persistent + storageCapacity: true + +--- +#### Controller Service +kind: Service +apiVersion: v1 +metadata: + name: csi-provisioner + namespace: kube-system + labels: + app: csi-provisioner +spec: + type: ClusterIP + clusterIP: None + selector: + app: csi-provisioner + +--- +kind: StatefulSet +apiVersion: apps/v1 +metadata: + name: csi-provisioner + namespace: kube-system +spec: + selector: + matchLabels: + app: csi-provisioner + serviceName: "csi-provisioner" + replicas: 1 + template: + metadata: + labels: + app: csi-provisioner + spec: + serviceAccountName: csi-provisioner + hostNetwork: true + containers: + - name: csi-provisioner + image: registry.k8s.io/sig-storage/csi-provisioner:v3.6.0 + imagePullPolicy: Always + args: + - "--csi-address=$(CSI_ENDPOINT)" + - "--timeout=60s" + - "--v=5" + env: + - name: CSI_ENDPOINT + value: /var/lib/csi/hs-csi.sock + volumeMounts: + - name: socket-dir + mountPath: /var/lib/csi/ + + - name: csi-attacher + image: registry.k8s.io/sig-storage/csi-attacher:v4.4.0 + imagePullPolicy: Always + args: + - "--csi-address=$(CSI_ENDPOINT)" + - "--v=5" + env: + - name: CSI_ENDPOINT + value: /var/lib/csi/hs-csi.sock + volumeMounts: + - name: socket-dir + mountPath: /var/lib/csi/ + + - name: csi-snapshotter + image: registry.k8s.io/sig-storage/csi-snapshotter:v6.2.1 + imagePullPolicy: Always + args: + - "--csi-address=$(CSI_ENDPOINT)" + - "--v=5" + env: + - name: CSI_ENDPOINT + value: /var/lib/csi/hs-csi.sock + volumeMounts: + - name: socket-dir + mountPath: /var/lib/csi/ + + - name: csi-resizer + image: registry.k8s.io/sig-storage/csi-resizer:v1.10.1 + imagePullPolicy: Always + args: + - "--csi-address=$(CSI_ENDPOINT)" + - "--v=5" + env: + - name: CSI_ENDPOINT + value: /var/lib/csi/hs-csi.sock + volumeMounts: + - name: socket-dir + mountPath: /var/lib/csi/ + + - name: hs-csi-plugin-controller + image: hammerspaceinc/csi-plugin:v1.2.9-rc1 + imagePullPolicy: Always + securityContext: + privileged: true + capabilities: + add: ["SYS_ADMIN"] + allowPrivilegeEscalation: true + envFrom: + - configMapRef: + name: csi-env-config + env: + - name: CSI_ENDPOINT + value: /var/lib/csi/hs-csi.sock + - name: HS_USERNAME + valueFrom: + secretKeyRef: + name: com.hammerspace.csi.credentials + key: username + - name: HS_PASSWORD + valueFrom: + secretKeyRef: + name: com.hammerspace.csi.credentials + key: password + - name: HS_ENDPOINT + valueFrom: + secretKeyRef: + name: com.hammerspace.csi.credentials + key: endpoint + - name: HS_TLS_VERIFY + value: "false" + - name: CSI_MAJOR_VERSION + value: "1" + volumeMounts: + - name: socket-dir + mountPath: /var/lib/csi/ + - name: staging-dir + mountPath: /var/lib/hammerspace/staging + + volumes: + - name: socket-dir + emptyDir: {} + - name: staging-dir + hostPath: + path: /var/lib/hammerspace/staging + type: DirectoryOrCreate + +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: csi-provisioner + namespace: kube-system + +--- +kind: ClusterRole +apiVersion: rbac.authorization.k8s.io/v1 +metadata: + name: csi-provisioner +rules: + - apiGroups: [""] + resources: ["pods", "nodes", "events", "endpoints"] + verbs: ["get", "list", "watch", "create", "update", "patch"] + - apiGroups: [""] + resources: ["persistentvolumes"] + verbs: ["get", "patch", "list", "watch", "create", "delete", "update"] + - apiGroups: [""] + resources: ["persistentvolumeclaims", "persistentvolumeclaims/status"] + verbs: ["get", "patch", "list", "watch", "update"] + - apiGroups: ["storage.k8s.io"] + resources: ["storageclasses", "volumeattachments", "volumeattachments/status"] + verbs: ["get", "list", "watch", "update", "patch"] + - apiGroups: ["snapshot.storage.k8s.io"] + resources: ["volumesnapshots", "volumesnapshotcontents", "volumesnapshotclasses"] + verbs: ["get", "list", "watch", "create", "update", "delete"] + - apiGroups: ["apiextensions.k8s.io"] + resources: ["customresourcedefinitions"] + verbs: ["create", "list", "watch", "delete"] + - apiGroups: [""] + resources: ["secrets"] + verbs: ["get", "list"] + +--- +kind: ClusterRoleBinding +apiVersion: rbac.authorization.k8s.io/v1 +metadata: + name: csi-provisioner +subjects: + - kind: ServiceAccount + name: csi-provisioner + namespace: kube-system +roleRef: + kind: ClusterRole + name: csi-provisioner + apiGroup: rbac.authorization.k8s.io + +--- +#### Node Service +kind: DaemonSet +apiVersion: apps/v1 +metadata: + name: csi-node + namespace: kube-system +spec: + selector: + matchLabels: + app: csi-node + template: + metadata: + labels: + app: csi-node + spec: + serviceAccountName: csi-node + hostNetwork: true + containers: + - name: driver-registrar + image: registry.k8s.io/sig-storage/csi-node-driver-registrar:v2.9.0 + imagePullPolicy: Always + args: + - "--v=5" + - "--csi-address=$(CSI_ENDPOINT)" + - "--kubelet-registration-path=$(REG_SOCKET)" + securityContext: + privileged: true + env: + - name: CSI_ENDPOINT + value: /csi/csi.sock + - name: REG_SOCKET + value: /var/lib/kubelet/plugins_registry/com.hammerspace.csi/csi.sock + - name: KUBE_NODE_NAME + valueFrom: + fieldRef: + fieldPath: spec.nodeName + volumeMounts: + - name: socket-dir + mountPath: /csi + - name: registration-dir + mountPath: /registration + + - name: hs-csi-plugin-node + image: hammerspaceinc/csi-plugin:v1.2.9-rc1 + imagePullPolicy: Always + securityContext: + privileged: true + capabilities: + add: ["SYS_ADMIN"] + allowPrivilegeEscalation: true + envFrom: + - configMapRef: + name: csi-env-config + env: + - name: CSI_ENDPOINT + value: /csi/csi.sock + - name: CSI_NODE_NAME + valueFrom: + fieldRef: + fieldPath: spec.nodeName + - name: HS_USERNAME + valueFrom: + secretKeyRef: + name: com.hammerspace.csi.credentials + key: username + - name: HS_PASSWORD + valueFrom: + secretKeyRef: + name: com.hammerspace.csi.credentials + key: password + - name: HS_ENDPOINT + valueFrom: + secretKeyRef: + name: com.hammerspace.csi.credentials + key: endpoint + - name: HS_TLS_VERIFY + value: "false" + - name: CSI_MAJOR_VERSION + value: "1" + volumeMounts: + - name: socket-dir + mountPath: /csi + - name: registration-dir + mountPath: /registration + mountPropagation: Bidirectional + - name: mountpoint-dir + mountPath: /var/lib/kubelet/ + mountPropagation: Bidirectional + - name: rootshare-dir + mountPath: /var/lib/hammerspace/ + mountPropagation: Bidirectional + - name: staging-dir + mountPath: /var/lib/hammerspace/staging + mountPropagation: Bidirectional + - name: dev-dir + mountPath: /dev + + volumes: + - name: socket-dir + hostPath: + path: /var/lib/kubelet/plugins_registry/com.hammerspace.csi + type: DirectoryOrCreate + - name: mountpoint-dir + hostPath: + path: /var/lib/kubelet/ + - name: rootshare-dir + hostPath: + path: /var/lib/hammerspace/ + type: DirectoryOrCreate + - name: staging-dir + hostPath: + path: /var/lib/hammerspace/staging + type: DirectoryOrCreate + - name: registration-dir + hostPath: + path: /var/lib/kubelet/plugins_registry/ + - name: dev-dir + hostPath: + path: /dev + +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: csi-node + namespace: kube-system + +--- +kind: ClusterRole +apiVersion: rbac.authorization.k8s.io/v1 +metadata: + name: csi-node +rules: + - apiGroups: [""] + resources: ["pods", "secrets", "nodes", "namespaces", "events"] + verbs: ["get", "list", "watch", "create", "update"] + - apiGroups: [""] + resources: ["persistentvolumes", "persistentvolumeclaims", "persistentvolumeclaims/status"] + verbs: ["get", "list", "watch", "update", "patch"] + - apiGroups: ["storage.k8s.io"] + resources: ["volumeattachments", "volumeattachments/status"] + verbs: ["get", "list", "watch", "update", "patch"] + +--- +kind: ClusterRoleBinding +apiVersion: rbac.authorization.k8s.io/v1 +metadata: + name: csi-node +subjects: + - kind: ServiceAccount + name: csi-node + namespace: kube-system +roleRef: + kind: ClusterRole + name: csi-node + apiGroup: rbac.authorization.k8s.io + +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: csi-env-config + namespace: kube-system +data: + MOUNT_CHECK_TIMEOUT: "30s" + UNMOUNT_RETRY_COUNT: "5" + UNMOUNT_RETRY_INTERVAL: "1s" \ No newline at end of file diff --git a/docs/multi-secret-sorageclass.md b/docs/multi-secret-sorageclass.md index 46a9fd7..8505a42 100644 --- a/docs/multi-secret-sorageclass.md +++ b/docs/multi-secret-sorageclass.md @@ -112,7 +112,9 @@ spec: ### Explanation: -- Secrets: The Secret resources store the credentials needed by the CSI driver to communicate with the Hammerspace backend. The stringData field is used to store the username, password, and CSI endpoint. +- Secrets: The Secret resources store the credentials used by the CSI driver to talk to the Hammerspace management/API endpoint (REST). These credentials are **not** used as NFS mount credentials; the data path mount is performed by the node via NFS, while the driver uses the API credentials to discover/prepare exports, create/delete shares, apply policies/objectives, etc. + - Required keys: `csiEndpoint`, `username`, `password` + - Optional keys: `csiTlsVerify` (`"true"`/`"false"`) - Storage Classes: The StorageClass resources define the type of storage to be provisioned. The parameters section includes: csi.storage.k8s.io/provisioner-secret-name: Specifies the name of the secret containing the credentials. @@ -123,7 +125,11 @@ Other storage-specific parameters (e.g., fsType, volumeNameFormat). #### Delete Behavior -DeleteVolume uses the provisioner secrets when provided. If the delete request does not include secrets, the driver will fall back to the secrets cached at CreateVolume time for the same volume ID. This cache is in-memory and will be lost if the controller restarts, so the recommended setup is to always supply provisioner secrets in the StorageClass. +DeleteVolume uses the provisioner secrets when provided. The driver does **not** cache secrets from CreateVolume for later use. + +If the delete request does not include secrets (for example, if the StorageClass is not configured with `csi.storage.k8s.io/provisioner-secret-name` / `csi.storage.k8s.io/provisioner-secret-namespace`), the driver falls back to the controller's configured credentials (for example, the Deployment's `com.hammerspace.csi.credentials` Secret via env vars). + +This is usually fine in single-endpoint clusters, but in multi-tenant or multi-endpoint setups it can cause share-backed deletes to fail if the controller's default credentials do not have access to the target Hammerspace system/share. In those setups, always supply provisioner secrets in the StorageClass so DeleteVolume is authenticated to the correct Hammerspace system. - PersistentVolumeClaims: The PersistentVolumeClaim resources request storage from the provisioner. The storageClassName field specifies which storage class to use, which in turn determines which secret will be used for provisioning. diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index ebe7b1c..a4e4765 100755 --- a/pkg/driver/controller.go +++ b/pkg/driver/controller.go @@ -818,9 +818,6 @@ func (d *CSIDriver) CreateVolume(ctx context.Context, req *csi.CreateVolumeReque } log.WithField("response", resp).Info("volume was created") - if volumeSecrets["csiEndpoint"] != "" { - d.storeVolumeSecrets(volID, volumeSecrets) - } return resp, nil } @@ -918,7 +915,7 @@ func (d *CSIDriver) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeReque } defer unlock() - hsclient, err := d.clientFromSecretsOrVolume(req.Secrets, volumeId) + hsclient, err := d.clientFromSecrets(req.Secrets) if err != nil { return nil, status.Errorf(codes.Internal, "failed to create hsclient from secrets, %v", err) } @@ -930,15 +927,9 @@ func (d *CSIDriver) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeReque } if share == nil { // Share does not exist, may be a file-backed volume err = d.deleteFileBackedVolume(ctx, hsclient, volumeId) - if err == nil { - d.deleteVolumeSecrets(volumeId) - } return &csi.DeleteVolumeResponse{}, err } else { // Share exists and is a Filesystem err = d.deleteShareBackedVolume(ctx, hsclient, share) - if err == nil { - d.deleteVolumeSecrets(volumeId) - } return &csi.DeleteVolumeResponse{}, err } diff --git a/pkg/driver/driver.go b/pkg/driver/driver.go index 90ca4c1..4846884 100644 --- a/pkg/driver/driver.go +++ b/pkg/driver/driver.go @@ -52,8 +52,6 @@ type CSIDriver struct { snapshotLocks map[string]*keyLock hsclient *client.HammerspaceClient NodeID string - secretsMu sync.RWMutex - volumeSecrets map[string]map[string]string nodeInfoMu sync.RWMutex nodeInfoKnown bool nodeIsPortal bool @@ -79,7 +77,6 @@ func NewCSIDriver(endpoint, username, password, tlsVerifyStr string) *CSIDriver volumeLocks: make(map[string]*keyLock), snapshotLocks: make(map[string]*keyLock), NodeID: os.Getenv("CSI_NODE_NAME"), - volumeSecrets: make(map[string]map[string]string), } } @@ -143,43 +140,6 @@ func (c *CSIDriver) acquireSnapshotLock(ctx context.Context, snapID string) (fun return func() { lk.unlock() }, nil } -func (d *CSIDriver) storeVolumeSecrets(volumeID string, secrets map[string]string) { - if volumeID == "" || len(secrets) == 0 { - return - } - dup := make(map[string]string, len(secrets)) - for k, v := range secrets { - dup[k] = v - } - d.secretsMu.Lock() - d.volumeSecrets[volumeID] = dup - d.secretsMu.Unlock() -} - -func (d *CSIDriver) deleteVolumeSecrets(volumeID string) { - if volumeID == "" { - return - } - d.secretsMu.Lock() - delete(d.volumeSecrets, volumeID) - d.secretsMu.Unlock() -} - -func (d *CSIDriver) clientFromSecretsOrVolume(secrets map[string]string, volumeID string) (*client.HammerspaceClient, error) { - if len(secrets) > 0 { - return client.NewHammerspaceClientFromSecrets(secrets) - } - if volumeID != "" { - d.secretsMu.RLock() - cached, ok := d.volumeSecrets[volumeID] - d.secretsMu.RUnlock() - if ok && len(cached) > 0 { - return client.NewHammerspaceClientFromSecrets(cached) - } - } - return d.hsclient, nil -} - func (c *CSIDriver) goServe(started chan<- bool) { c.wg.Add(1) go func() {