From 1b6944106ec4bb402e671d2b8be376e0df0a27a3 Mon Sep 17 00:00:00 2001 From: nijayf Date: Mon, 3 Nov 2025 17:00:14 +0530 Subject: [PATCH 1/5] Release v2.15.1 changes --- pkg/controller/controller.go | 49 +++++---- pkg/controller/controller_test.go | 164 +++++++++++++++++++++--------- pkg/controller/creator.go | 15 ++- pkg/controller/creator_test.go | 2 + 4 files changed, 162 insertions(+), 68 deletions(-) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 06e003fe..80170fac 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -440,26 +440,42 @@ func (s *Service) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest params[identifiers.KeyVolumeDescription] = getDescription(req.GetParameters()) var volumeResponse *csi.Volume - resp, createError := creator.Create(ctx, req, sizeInBytes, arr.GetClient()) - if createError != nil { - log.Warnf("create volume for %s failed: '%s'", req.GetName(), createError.Error()) - if apiError, ok := createError.(gopowerstore.APIError); ok && (apiError.VolumeNameIsAlreadyUse() || apiError.FSNameIsAlreadyUse()) { - volumeResponse, err = creator.CheckIfAlreadyExists(ctx, req.GetName(), sizeInBytes, arr.GetClient()) - if err != nil { - if useNFS && status.Code(err) != codes.AlreadyExists { - arr.NASCooldownTracker.MarkFailure(selectedNasName) - return nil, status.Error(codes.ResourceExhausted, createError.Error()) - } - return nil, err - } - } else { + + + + // check if job is already in progress on array, if so, return error and let CO check again + if useNFS { + jobs, err := arr.Client.GetInProgressJobsByFsName(ctx, req.GetName()) + if err != nil { + log.Errorf("Error getting jobs that are in progress for FileSystem: %s error: %s", req.Name, err.Error()) + return nil, status.Errorf(codes.Internal, "Error getting jobs that are in progress for FileSystem: %s error: %s", req.Name, err.Error()) + } + if len(jobs) > 0 { + log.Infof("Job already in progress to create FileSystem %s", req.GetName()) + return nil, status.Errorf(codes.AlreadyExists, "Job already in progress to create FileSystem %s", req.GetName()) + } + } + + // check if vol exists before creating it in the array + volumeResponse, err = creator.CheckIfAlreadyExists(ctx, req.GetName(), sizeInBytes, arr.GetClient()) + if err != nil { + // internal means something went wrong trying to check the volume and request needs to be retried + if status.Code(err) == codes.Internal || status.Code(err) == codes.AlreadyExists { + log.Warnf("CheckIfAlreadyExists returned error: %s for vol: %s", err.Error(), req.GetName()) + return nil, err + } + } + + if volumeResponse == nil { + resp, createError := creator.Create(ctx, req, sizeInBytes, arr.GetClient()) + if createError != nil { + log.Warnf("create volume for %s failed: '%s'", req.GetName(), createError.Error()) if useNFS { arr.NASCooldownTracker.MarkFailure(selectedNasName) return nil, status.Error(codes.ResourceExhausted, createError.Error()) } - return nil, status.Error(codes.Internal, createError.Error()) + return nil, createError } - } else { if useNFS { arr.NASCooldownTracker.ResetFailure(selectedNasName) } @@ -501,7 +517,6 @@ func (s *Service) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest // Fetch the service tag serviceTag := GetServiceTag(ctx, req, arr, volumeResponse.VolumeId, protocol) - volumeResponse.VolumeContext = req.Parameters volumeResponse.VolumeContext[identifiers.KeyArrayID] = arr.GetGlobalID() volumeResponse.VolumeContext[identifiers.KeyArrayVolumeName] = req.Name @@ -510,7 +525,7 @@ func (s *Service) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest if useNFS { volumeResponse.VolumeContext[identifiers.KeyNfsACL] = nfsAcls - volumeResponse.VolumeContext[identifiers.KeyNasName] = selectedNasName + volumeResponse.VolumeContext[identifiers.KeyNasName] = creator.(*NfsCreator).nasName topology = identifiers.GetNfsTopology(arr.GetIP()) log.Infof("Modified topology to nfs for %s", req.GetName()) } diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index c15441c0..5140c851 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -202,6 +202,8 @@ var _ = ginkgo.Describe("CSIControllerService", func() { ginkgo.When("creating block volume", func() { ginkgo.It("should successfully create block volume", func() { clientMock.On("GetCustomHTTPHeaders").Return(api.NewSafeHeader().GetHeader()) + clientMock.On("GetVolumeByName", mock.Anything, mock.Anything).Return( + gopowerstore.Volume{}, errors.New("no vol found")) clientMock.On("GetSoftwareMajorMinorVersion", context.Background()).Return(float32(3.0), nil) clientMock.On("SetCustomHTTPHeaders", mock.Anything).Return(nil) clientMock.On("CreateVolume", mock.Anything, mock.Anything).Return(gopowerstore.CreateResponse{ID: validBaseVolID}, nil) @@ -237,6 +239,8 @@ var _ = ginkgo.Describe("CSIControllerService", func() { clientMock.On("GetCustomHTTPHeaders").Return(api.NewSafeHeader().GetHeader()) clientMock.On("GetSoftwareMajorMinorVersion", context.Background()).Return(float32(3.0), nil) clientMock.On("SetCustomHTTPHeaders", mock.Anything).Return(nil) + clientMock.On("GetVolumeByName", mock.Anything, mock.Anything).Return( + gopowerstore.Volume{}, errors.New("no vol found")) clientMock.On("CreateVolume", mock.Anything, mock.Anything).Return(gopowerstore.CreateResponse{ID: validBaseVolID}, nil) clientMock.On("GetVolume", context.Background(), mock.Anything).Return(gopowerstore.Volume{ApplianceID: validApplianceID}, nil) clientMock.On("GetAppliance", context.Background(), mock.Anything).Return(gopowerstore.ApplianceInstance{ServiceTag: validServiceTag}, nil) @@ -292,6 +296,8 @@ var _ = ginkgo.Describe("CSIControllerService", func() { ginkgo.It("should create volume and volumeGroup if policy exists - ASYNC", func() { clientMock.On("GetCustomHTTPHeaders").Return(api.NewSafeHeader().GetHeader()) + clientMock.On("GetVolumeByName", mock.Anything, mock.Anything).Return( + gopowerstore.Volume{}, errors.New("no vol found")) clientMock.On("GetSoftwareMajorMinorVersion", context.Background()).Return(float32(3.0), nil) clientMock.On("SetCustomHTTPHeaders", mock.Anything).Return(nil) // all entities not exists @@ -334,6 +340,8 @@ var _ = ginkgo.Describe("CSIControllerService", func() { ginkgo.It("should create volume and volumeGroup if policy exists - SYNC", func() { clientMock.On("GetCustomHTTPHeaders").Return(api.NewSafeHeader().GetHeader()) + clientMock.On("GetVolumeByName", mock.Anything, mock.Anything).Return( + gopowerstore.Volume{}, errors.New("no vol found")) clientMock.On("GetSoftwareMajorMinorVersion", context.Background()).Return(float32(3.0), nil) clientMock.On("SetCustomHTTPHeaders", mock.Anything).Return(nil) // Setting Replciation mode and corresponding attributes for SYNC @@ -389,6 +397,9 @@ var _ = ginkgo.Describe("CSIControllerService", func() { req.Parameters[KeyCSIPVCNamespace] = "" }() + clientMock.On("GetVolumeByName", mock.Anything, mock.Anything).Return( + gopowerstore.Volume{}, errors.New("no vol found")) + clientMock.On("GetVolumeGroupByName", mock.Anything, validNamespacedGroupName). Return(gopowerstore.VolumeGroup{}, gopowerstore.NewNotFoundError()) @@ -447,6 +458,9 @@ var _ = ginkgo.Describe("CSIControllerService", func() { req.Parameters[KeyCSIPVCNamespace] = "" }() + clientMock.On("GetVolumeByName", mock.Anything, mock.Anything).Return( + gopowerstore.Volume{}, errors.New("no vol found")) + clientMock.On("GetVolumeGroupByName", mock.Anything, validNamespacedGroupNameSync). Return(gopowerstore.VolumeGroup{}, gopowerstore.NewNotFoundError()) @@ -494,6 +508,8 @@ var _ = ginkgo.Describe("CSIControllerService", func() { }) ginkgo.It("should create new volume with existing volumeGroup with policy - ASYNC", func() { + clientMock.On("GetVolumeByName", mock.Anything, mock.Anything).Return( + gopowerstore.Volume{}, errors.New("no vol found")) clientMock.On("GetVolumeGroupByName", mock.Anything, validGroupName). Return(gopowerstore.VolumeGroup{ID: validGroupID, ProtectionPolicyID: validPolicyID}, nil) @@ -531,6 +547,8 @@ var _ = ginkgo.Describe("CSIControllerService", func() { }) ginkgo.It("should create new volume with existing volumeGroup with policy - SYNC", func() { + clientMock.On("GetVolumeByName", mock.Anything, mock.Anything).Return( + gopowerstore.Volume{}, errors.New("no vol found")) clientMock.On("GetVolumeGroupByName", mock.Anything, validGroupNameSync). Return(gopowerstore.VolumeGroup{ID: validGroupID, ProtectionPolicyID: validPolicyID, IsWriteOrderConsistent: true}, nil) // Setting Replciation mode and corresponding attributes for SYNC @@ -594,6 +612,8 @@ var _ = ginkgo.Describe("CSIControllerService", func() { }) ginkgo.It("should create volume and update volumeGroup without policy, but policy exists - ASYNC", func() { + clientMock.On("GetVolumeByName", mock.Anything, mock.Anything).Return( + gopowerstore.Volume{}, errors.New("no vol found")) clientMock.On("GetVolumeGroupByName", mock.Anything, validGroupName). Return(gopowerstore.VolumeGroup{ID: validGroupID, ProtectionPolicyID: validPolicyID}, nil) @@ -633,6 +653,8 @@ var _ = ginkgo.Describe("CSIControllerService", func() { }) ginkgo.It("should create volume and update volumeGroup without policy, but policy exists - SYNC", func() { + clientMock.On("GetVolumeByName", mock.Anything, mock.Anything).Return( + gopowerstore.Volume{}, errors.New("no vol found")) clientMock.On("GetVolumeGroupByName", mock.Anything, validGroupNameSync). Return(gopowerstore.VolumeGroup{ID: validGroupID, ProtectionPolicyID: validPolicyID, IsWriteOrderConsistent: true}, nil) @@ -751,6 +773,8 @@ var _ = ginkgo.Describe("CSIControllerService", func() { ginkgo.It("should default RPO to Zero when mode is SYNC and RPO is not specified", func() { delete(req.Parameters, ctrlSvc.WithRP(KeyReplicationRPO)) + clientMock.On("GetVolumeByName", mock.Anything, mock.Anything).Return( + gopowerstore.Volume{}, errors.New("no vol found")) clientMock.On("GetVolumeGroupByName", mock.Anything, validGroupNameSync). Return(gopowerstore.VolumeGroup{ID: validGroupID, ProtectionPolicyID: validPolicyID, IsWriteOrderConsistent: true}, nil) @@ -877,6 +901,8 @@ var _ = ginkgo.Describe("CSIControllerService", func() { delete(req.Parameters, ctrlSvc.WithRP(KeyReplicationIgnoreNamespaces)) delete(req.Parameters, ctrlSvc.WithRP(KeyReplicationVGPrefix)) + clientMock.On("GetVolumeByName", mock.Anything, mock.Anything).Return( + gopowerstore.Volume{}, errors.New("no vol found")) clientMock.On("ConfigureMetroVolume", mock.Anything, validBaseVolID, configureMetroRequest). Return(gopowerstore.MetroSessionResponse{ID: validSessionID}, nil) clientMock.On("GetVolume", context.Background(), mock.Anything). @@ -916,6 +942,9 @@ var _ = ginkgo.Describe("CSIControllerService", func() { delete(req.Parameters, ctrlSvc.WithRP(KeyReplicationIgnoreNamespaces)) delete(req.Parameters, ctrlSvc.WithRP(KeyReplicationVGPrefix)) + clientMock.On("GetVolumeByName", mock.Anything, mock.Anything).Return( + gopowerstore.Volume{}, errors.New("no vol found")) + clientMock.On("ConfigureMetroVolume", mock.Anything, validBaseVolID, configureMetroRequest). Return(gopowerstore.MetroSessionResponse{}, gopowerstore.APIError{ ErrorMsg: &api.ErrorMsg{ @@ -955,6 +984,8 @@ var _ = ginkgo.Describe("CSIControllerService", func() { }) ginkgo.It("should fail to configure metro replication on volume if the volume cannot be found", func() { + clientMock.On("GetVolumeByName", mock.Anything, mock.Anything).Return( + gopowerstore.Volume{}, errors.New("no vol found")) // Return volume not found error when trying to configure a metro session for that volume clientMock.On("ConfigureMetroVolume", mock.Anything, validBaseVolID, configureMetroRequest). Return(gopowerstore.MetroSessionResponse{}, gopowerstore.NewNotFoundError()) @@ -981,6 +1012,8 @@ var _ = ginkgo.Describe("CSIControllerService", func() { }) ginkgo.It("should fail if it can't find the replication session", func() { + clientMock.On("GetVolumeByName", mock.Anything, mock.Anything).Return( + gopowerstore.Volume{}, errors.New("no vol found")) clientMock.On("ConfigureMetroVolume", mock.Anything, validBaseVolID, configureMetroRequest). Return(gopowerstore.MetroSessionResponse{ID: validSessionID}, nil) clientMock.On("GetVolume", context.Background(), mock.Anything). @@ -997,6 +1030,8 @@ var _ = ginkgo.Describe("CSIControllerService", func() { }) ginkgo.It("should fail if the replication session resource type is incorrect", func() { + clientMock.On("GetVolumeByName", mock.Anything, mock.Anything).Return( + gopowerstore.Volume{}, errors.New("no vol found")) clientMock.On("ConfigureMetroVolume", mock.Anything, validBaseVolID, configureMetroRequest). Return(gopowerstore.MetroSessionResponse{ID: validSessionID}, nil) clientMock.On("GetVolume", context.Background(), mock.Anything). @@ -1024,6 +1059,8 @@ var _ = ginkgo.Describe("CSIControllerService", func() { clientMock.On("GetFS", context.Background(), mock.Anything).Return(gopowerstore.FileSystem{NasServerID: validNasID}, nil) clientMock.On("GetNAS", context.Background(), mock.Anything).Return(gopowerstore.NAS{CurrentNodeID: validNodeID}, nil) clientMock.On("GetApplianceByName", context.Background(), mock.Anything).Return(gopowerstore.ApplianceInstance{ServiceTag: validServiceTag}, nil) + clientMock.On("GetInProgressJobsByFsName", context.Background(), mock.Anything).Return([]gopowerstore.Job{}, nil) + clientMock.On("GetFSByName", mock.Anything, mock.Anything).Return(gopowerstore.FileSystem{}, errors.New("not found")) req := getTypicalCreateVolumeNFSRequest("my-vol", validVolSize) req.Parameters[identifiers.KeyArrayID] = secondValidID @@ -1059,6 +1096,8 @@ var _ = ginkgo.Describe("CSIControllerService", func() { clientMock.On("GetFS", context.Background(), mock.Anything).Return(gopowerstore.FileSystem{NasServerID: validNasID}, nil) clientMock.On("GetNAS", context.Background(), mock.Anything).Return(gopowerstore.NAS{CurrentNodeID: validNodeID}, nil) clientMock.On("GetApplianceByName", context.Background(), mock.Anything).Return(gopowerstore.ApplianceInstance{ServiceTag: validServiceTag}, nil) + clientMock.On("GetInProgressJobsByFsName", context.Background(), mock.Anything).Return([]gopowerstore.Job{}, nil) + clientMock.On("GetFSByName", mock.Anything, mock.Anything).Return(gopowerstore.FileSystem{}, errors.New("not found")) req := getTypicalCreateVolumeNFSRequest("my-vol", validVolSize) req.Parameters[identifiers.KeyArrayID] = secondValidID @@ -1121,6 +1160,8 @@ var _ = ginkgo.Describe("CSIControllerService", func() { clientMock.On("GetFS", context.Background(), mock.Anything).Return(gopowerstore.FileSystem{NasServerID: validNasID}, nil) clientMock.On("GetNAS", context.Background(), mock.Anything).Return(gopowerstore.NAS{CurrentNodeID: validNodeID}, nil) clientMock.On("GetApplianceByName", context.Background(), mock.Anything).Return(gopowerstore.ApplianceInstance{ServiceTag: validServiceTag}, nil) + clientMock.On("GetInProgressJobsByFsName", context.Background(), mock.Anything).Return([]gopowerstore.Job{}, nil) + clientMock.On("GetFSByName", mock.Anything, mock.Anything).Return(gopowerstore.FileSystem{}, errors.New("not found")) ctrlSvc.Arrays()[secondValidID].NfsAcls = "A::GROUP@:RWX" @@ -1161,6 +1202,8 @@ var _ = ginkgo.Describe("CSIControllerService", func() { clientMock.On("GetFS", context.Background(), mock.Anything).Return(gopowerstore.FileSystem{NasServerID: validNasID}, nil) clientMock.On("GetNAS", context.Background(), mock.Anything).Return(gopowerstore.NAS{CurrentNodeID: validNodeID}, nil) clientMock.On("GetApplianceByName", context.Background(), mock.Anything).Return(gopowerstore.ApplianceInstance{ServiceTag: validServiceTag}, nil) + clientMock.On("GetInProgressJobsByFsName", context.Background(), mock.Anything).Return([]gopowerstore.Job{}, nil) + clientMock.On("GetFSByName", mock.Anything, mock.Anything).Return(gopowerstore.FileSystem{}, errors.New("not found")) ctrlSvc.Arrays()[secondValidID].NfsAcls = "A::GROUP@:RWX" @@ -1200,6 +1243,8 @@ var _ = ginkgo.Describe("CSIControllerService", func() { clientMock.On("GetFS", context.Background(), mock.Anything).Return(gopowerstore.FileSystem{NasServerID: validNasID}, nil) clientMock.On("GetNAS", context.Background(), mock.Anything).Return(gopowerstore.NAS{CurrentNodeID: validNodeID}, nil) clientMock.On("GetApplianceByName", context.Background(), mock.Anything).Return(gopowerstore.ApplianceInstance{ServiceTag: validServiceTag}, nil) + clientMock.On("GetInProgressJobsByFsName", context.Background(), mock.Anything).Return([]gopowerstore.Job{}, nil) + clientMock.On("GetFSByName", mock.Anything, mock.Anything).Return(gopowerstore.FileSystem{}, errors.New("not found")) req := getTypicalCreateVolumeNFSRequest("my-vol", validVolSize) req.Parameters[identifiers.KeyArrayID] = secondValidID @@ -1237,6 +1282,8 @@ var _ = ginkgo.Describe("CSIControllerService", func() { clientMock.On("GetFS", context.Background(), mock.Anything).Return(gopowerstore.FileSystem{NasServerID: validNasID}, nil) clientMock.On("GetNAS", context.Background(), mock.Anything).Return(gopowerstore.NAS{CurrentNodeID: validNodeID}, nil) clientMock.On("GetApplianceByName", context.Background(), mock.Anything).Return(gopowerstore.ApplianceInstance{ServiceTag: validServiceTag}, nil) + clientMock.On("GetInProgressJobsByFsName", context.Background(), mock.Anything).Return([]gopowerstore.Job{}, nil) + clientMock.On("GetFSByName", mock.Anything, mock.Anything).Return(gopowerstore.FileSystem{}, errors.New("not found")) req := getTypicalCreateVolumeNFSRequest("my-vol", validVolSize) req.Parameters[identifiers.KeyArrayID] = secondValidID @@ -1299,6 +1346,8 @@ var _ = ginkgo.Describe("CSIControllerService", func() { clientMock.On("GetFS", context.Background(), mock.Anything).Return(gopowerstore.FileSystem{NasServerID: validNasID}, nil) clientMock.On("GetNAS", context.Background(), mock.Anything).Return(gopowerstore.NAS{CurrentNodeID: validNodeID}, nil) clientMock.On("GetApplianceByName", context.Background(), mock.Anything).Return(gopowerstore.ApplianceInstance{ServiceTag: validServiceTag}, nil) + clientMock.On("GetInProgressJobsByFsName", context.Background(), mock.Anything).Return([]gopowerstore.Job{}, nil) + clientMock.On("GetFSByName", mock.Anything, mock.Anything).Return(gopowerstore.FileSystem{}, errors.New("not found")) req := getTypicalCreateVolumeNFSRequest("my-vol", validVolSize) req.Parameters[identifiers.KeyArrayID] = secondValidID @@ -1393,8 +1442,18 @@ var _ = ginkgo.Describe("CSIControllerService", func() { SizeTotal: validVolSize, }, nil) clientMock.On("GetFS", context.Background(), mock.Anything).Return(gopowerstore.FileSystem{NasServerID: validNasID}, nil) - clientMock.On("GetNAS", context.Background(), mock.Anything).Return(gopowerstore.NAS{CurrentNodeID: validNodeID}, nil) + clientMock.On("GetNAS", mock.Anything, mock.Anything). + Return(gopowerstore.NAS{ + Name: validNasName, + CurrentNodeID: validNodeID, + }, nil) clientMock.On("GetApplianceByName", context.Background(), mock.Anything).Return(gopowerstore.ApplianceInstance{ServiceTag: validServiceTag}, nil) + clientMock.On("GetInProgressJobsByFsName", context.Background(), mock.Anything).Return([]gopowerstore.Job{}, nil) + clientMock.On("GetFSByName", mock.Anything, volName).Return(gopowerstore.FileSystem{ + ID: validBaseVolID, + Name: volName, + SizeTotal: validVolSize, + }, nil) req := getTypicalCreateVolumeNFSRequest(volName, validVolSize) req.Parameters[identifiers.KeyArrayID] = secondValidID @@ -1456,7 +1515,7 @@ var _ = ginkgo.Describe("CSIControllerService", func() { ginkgo.It("should fail [NFS]", func() { volName := "my-vol" clientMock.On("GetNASByName", mock.Anything, validNasName).Return(gopowerstore.NAS{ID: validNasID}, nil) - + clientMock.On("GetInProgressJobsByFsName", context.Background(), mock.Anything).Return([]gopowerstore.Job{}, nil) clientMock.On("CreateFS", mock.Anything, mock.Anything).Return(gopowerstore.CreateResponse{}, gopowerstore.APIError{ ErrorMsg: &api.ErrorMsg{ StatusCode: http.StatusUnprocessableEntity, @@ -1693,6 +1752,9 @@ var _ = ginkgo.Describe("CSIControllerService", func() { ginkgo.When("there is no array IP in storage class", func() { ginkgo.It("should use default array", func() { clientMock.On("GetCustomHTTPHeaders").Return(api.NewSafeHeader().GetHeader()) + clientMock.On("GetVolumeByName", mock.Anything, mock.Anything).Return( + gopowerstore.Volume{}, errors.New("no vol found")) + clientMock.On("GetInProgressJobsByFsName", context.Background(), mock.Anything).Return([]gopowerstore.Job{}, nil) clientMock.On("GetSoftwareMajorMinorVersion", context.Background()).Return(float32(3.0), nil) clientMock.On("SetCustomHTTPHeaders", mock.Anything).Return(nil) clientMock.On("CreateVolume", mock.Anything, mock.Anything).Return(gopowerstore.CreateResponse{ID: validBaseVolID}, nil) @@ -1838,6 +1900,8 @@ var _ = ginkgo.Describe("CSIControllerService", func() { clientMock.On("GetFS", context.Background(), mock.Anything).Return(gopowerstore.FileSystem{NasServerID: validNasID}, nil) clientMock.On("GetNAS", context.Background(), mock.Anything).Return(gopowerstore.NAS{CurrentNodeID: validNodeID}, nil) clientMock.On("GetApplianceByName", context.Background(), mock.Anything).Return(gopowerstore.ApplianceInstance{ServiceTag: validServiceTag}, nil) + clientMock.On("GetInProgressJobsByFsName", context.Background(), mock.Anything).Return([]gopowerstore.Job{}, nil) + clientMock.On("GetFSByName", mock.Anything, mock.Anything).Return(gopowerstore.FileSystem{}, errors.New("not found")) req := getTypicalCreateVolumeNFSRequest("my-vol", validVolSize) req.Parameters[identifiers.KeyArrayID] = secondValidID @@ -1874,6 +1938,8 @@ var _ = ginkgo.Describe("CSIControllerService", func() { clientMock.On("GetFS", context.Background(), mock.Anything).Return(gopowerstore.FileSystem{NasServerID: validNasID}, nil) clientMock.On("GetNAS", context.Background(), mock.Anything).Return(gopowerstore.NAS{CurrentNodeID: validNodeID}, nil) clientMock.On("GetApplianceByName", context.Background(), mock.Anything).Return(gopowerstore.ApplianceInstance{ServiceTag: validServiceTag}, nil) + clientMock.On("GetInProgressJobsByFsName", context.Background(), mock.Anything).Return([]gopowerstore.Job{}, nil) + clientMock.On("GetFSByName", mock.Anything, mock.Anything).Return(gopowerstore.FileSystem{}, errors.New("not found")) req := getTypicalCreateVolumeNFSRequest("my-vol", validVolSize) req.Parameters[identifiers.KeyArrayID] = secondValidID @@ -1920,6 +1986,7 @@ var _ = ginkgo.Describe("CSIControllerService", func() { }) ginkgo.It("should fail if CreateFS fails with NAS limit error & failure count should be incremented", func() { + clientMock.On("GetInProgressJobsByFsName", context.Background(), mock.Anything).Return([]gopowerstore.Job{}, nil) clientMock.On("GetNASServers", mock.Anything).Return([]gopowerstore.NAS{validNAS1, validNAS2, validNAS3, invalidNAS4}, nil) clientMock.On("GetNASByName", mock.Anything, "nasA").Return(gopowerstore.NAS{ID: validNasID}, nil) clientMock.On("CreateFS", mock.Anything, mock.Anything). @@ -1930,6 +1997,7 @@ var _ = ginkgo.Describe("CSIControllerService", func() { }, }) clientMock.On("GetFSByName", mock.Anything, "my-vol").Return(gopowerstore.FileSystem{}, errors.New("not nil")) + clientMock.On("GetInProgressJobsByFsName", context.Background(), mock.Anything).Return([]gopowerstore.Job{}, nil) req := getTypicalCreateVolumeNFSRequest("my-vol", validVolSize) req.Parameters[identifiers.KeyArrayID] = secondValidID @@ -1955,6 +2023,8 @@ var _ = ginkgo.Describe("CSIControllerService", func() { Message: "some error message", }, }) + clientMock.On("GetInProgressJobsByFsName", context.Background(), mock.Anything).Return([]gopowerstore.Job{}, nil) + clientMock.On("GetFSByName", mock.Anything, mock.Anything).Return(gopowerstore.FileSystem{}, errors.New("not found")) req := getTypicalCreateVolumeNFSRequest("my-vol", validVolSize) req.Parameters[identifiers.KeyArrayID] = secondValidID @@ -1971,6 +2041,9 @@ var _ = ginkgo.Describe("CSIControllerService", func() { }) ginkgo.It("should fail if CreateFS fails with Non-API error & failure count should be incremented", func() { + clientMock.On("GetInProgressJobsByFsName", context.Background(), mock.Anything).Return([]gopowerstore.Job{}, nil) + clientMock.On("GetFSByName", mock.Anything, mock.Anything).Return( + gopowerstore.FileSystem{}, errors.New("no vol found")) clientMock.On("GetNASServers", mock.Anything).Return([]gopowerstore.NAS{validNAS1, validNAS2, validNAS3, invalidNAS4}, nil) clientMock.On("GetNASByName", mock.Anything, "nasA").Return(gopowerstore.NAS{ID: validNasID}, nil) clientMock.On("CreateFS", mock.Anything, mock.Anything).Return(gopowerstore.CreateResponse{}, errors.New("some error message")) @@ -1989,7 +2062,46 @@ var _ = ginkgo.Describe("CSIControllerService", func() { gomega.Expect(tracker.GetStatusMap()["nasA"].Failures).To(gomega.Equal(1)) }) + ginkgo.It("should fail when listing jobs returns error [NFS]", func() { + volName := "my-vol" + clientMock.On("GetNASByName", mock.Anything, validNasName).Return(gopowerstore.NAS{ID: validNasID}, nil) + clientMock.On("GetInProgressJobsByFsName", context.Background(), mock.Anything).Return([]gopowerstore.Job{}, errors.New("cannot list jobs")) + req := getTypicalCreateVolumeNFSRequest(volName, validVolSize) + req.Parameters[identifiers.KeyArrayID] = secondValidID + res, err := ctrlSvc.CreateVolume(context.Background(), req) + + gomega.Expect(res).To(gomega.BeNil()) + gomega.Expect(err).ToNot(gomega.BeNil()) + gomega.Expect(err.Error()).To( + gomega.ContainSubstring("Error getting jobs that are in progress"), + ) + }) + + ginkgo.It("should fail when an in progress job is found- to prevent duplicate volumes", func() { + volName := "my-vol" + clientMock.On("GetNASByName", mock.Anything, validNasName).Return(gopowerstore.NAS{ID: validNasID}, nil) + clientMock.On("GetInProgressJobsByFsName", context.Background(), mock.Anything).Return([]gopowerstore.Job{ + { + ID: "1", + Action: "create", + Type: "file_system", + ResourceName: volName, + State: "InProgress", + }, + }, nil) + req := getTypicalCreateVolumeNFSRequest(volName, validVolSize) + req.Parameters[identifiers.KeyArrayID] = secondValidID + res, err := ctrlSvc.CreateVolume(context.Background(), req) + + gomega.Expect(res).To(gomega.BeNil()) + gomega.Expect(err).ToNot(gomega.BeNil()) + gomega.Expect(err.Error()).To( + gomega.ContainSubstring("Job already in progress"), + ) + }) + ginkgo.It("should enter a cooldown if the failure threshold (5) is reached & fallback to next best nas server", func() { + clientMock.On("GetInProgressJobsByFsName", context.Background(), mock.Anything).Return([]gopowerstore.Job{}, nil) clientMock.On("GetNASServers", mock.Anything).Return([]gopowerstore.NAS{validNAS1, validNAS2, validNAS3, invalidNAS4}, nil) // 1st to 5th call: return nasA clientMock.On("GetNASByName", mock.Anything, "nasA").Return(gopowerstore.NAS{ID: validNasID}, nil).Times(5) @@ -2039,6 +2151,8 @@ var _ = ginkgo.Describe("CSIControllerService", func() { clientMock.On("CreateFS", mock.Anything, mock.Anything). Return(gopowerstore.CreateResponse{}, errors.New("some error message")).Once() + clientMock.On("GetInProgressJobsByFsName", context.Background(), mock.Anything).Return([]gopowerstore.Job{}, nil) + clientMock.On("GetFSByName", mock.Anything, mock.Anything).Return(gopowerstore.FileSystem{}, errors.New("not found")) req := getTypicalCreateVolumeNFSRequest("my-vol", validVolSize) req.Parameters[identifiers.KeyArrayID] = secondValidID @@ -5285,52 +5399,6 @@ var _ = ginkgo.Describe("CSIControllerService", func() { })) }) - ginkgo.It("should successfully discover protection group of a host-based nfs volume", func() { - clientMock.On("GetVolumeGroupsByVolumeID", mock.Anything, validBaseVolID). - Return(gopowerstore.VolumeGroups{VolumeGroup: []gopowerstore.VolumeGroup{{ID: validGroupID, Name: validVolumeGroupName}}}, nil) - - clientMock.On("GetReplicationSessionByLocalResourceID", mock.Anything, validGroupID). - Return(gopowerstore.ReplicationSession{ - RemoteSystemID: validRemoteSystemID, - LocalResourceID: validGroupID, - RemoteResourceID: validRemoteGroupID, - StorageElementPairs: []gopowerstore.StorageElementPair{{ - LocalStorageElementID: validBaseVolID, - RemoteStorageElementID: validRemoteVolID, - }}, - }, nil) - - clientMock.On("GetCluster", mock.Anything). - Return(gopowerstore.Cluster{Name: validClusterName, ManagementAddress: firstValidID}, nil) - - clientMock.On("GetRemoteSystem", mock.Anything, validRemoteSystemID). - Return(gopowerstore.RemoteSystem{ - Name: validRemoteSystemName, - ManagementAddress: secondValidID, - SerialNumber: validRemoteSystemGlobalID, - }, nil) - - req := &csiext.CreateStorageProtectionGroupRequest{ - VolumeHandle: nfs.CsiNfsPrefixDash + validBaseVolID + "/" + firstValidID + "/" + "iscsi", - Parameters: map[string]string{ - nfs.CsiNfsParameter: "RWX", - }, - } - - res, err := ctrlSvc.CreateStorageProtectionGroup(context.Background(), req) - - localParams, remoteParams := getLocalAndRemoteParams(validClusterName, firstValidID, - validRemoteSystemName, secondValidID, validRemoteSystemGlobalID, validVolumeGroupName) - - gomega.Expect(err).To(gomega.BeNil()) - gomega.Expect(res).To(gomega.Equal(&csiext.CreateStorageProtectionGroupResponse{ - LocalProtectionGroupId: validGroupID, - RemoteProtectionGroupId: validRemoteGroupID, - LocalProtectionGroupAttributes: localParams, - RemoteProtectionGroupAttributes: remoteParams, - })) - }) - ginkgo.It("should fail if volume doesn't exists", func() { req := &csiext.CreateStorageProtectionGroupRequest{ VolumeHandle: "", diff --git a/pkg/controller/creator.go b/pkg/controller/creator.go index 76788104..deca43c0 100644 --- a/pkg/controller/creator.go +++ b/pkg/controller/creator.go @@ -213,7 +213,7 @@ func (*SCSICreator) CheckName(_ context.Context, name string) error { func (*SCSICreator) CheckIfAlreadyExists(ctx context.Context, name string, sizeInBytes int64, client gopowerstore.Client) (*csi.Volume, error) { alreadyExistVolume, err := client.GetVolumeByName(ctx, name) if err != nil { - return nil, status.Errorf(codes.Internal, "can't find volume '%s': %s", name, err.Error()) + return nil, status.Errorf(status.Code(err), "can't find volume '%s': %s", name, err.Error()) } if alreadyExistVolume.Size < sizeInBytes { @@ -408,10 +408,10 @@ func (*NfsCreator) CheckName(_ context.Context, name string) error { } // CheckIfAlreadyExists queries storage array if FileSystem with given name exists -func (*NfsCreator) CheckIfAlreadyExists(ctx context.Context, name string, sizeInBytes int64, client gopowerstore.Client) (*csi.Volume, error) { +func (c *NfsCreator) CheckIfAlreadyExists(ctx context.Context, name string, sizeInBytes int64, client gopowerstore.Client) (*csi.Volume, error) { alreadyExistVolume, err := client.GetFSByName(ctx, name) if err != nil { - return nil, status.Errorf(codes.Internal, "can't find filesystem '%s': %s", name, err.Error()) + return nil, status.Errorf(status.Code(err), "can't find filesystem '%s': %s", name, err.Error()) } if alreadyExistVolume.SizeTotal < sizeInBytes { @@ -420,6 +420,15 @@ func (*NfsCreator) CheckIfAlreadyExists(ctx context.Context, name string, sizeIn name, alreadyExistVolume.SizeTotal, sizeInBytes) } log.Infof("filesystem '%s' already exists", name) + + + // update the nas server name for the volume to ensure CreateVolume adds the correct nas to volume context + nasServerID := alreadyExistVolume.NasServerID + nas, err := client.GetNAS(ctx, nasServerID) + if err != nil { + return nil, status.Errorf(codes.Internal, "can't find nas server '%s': %s", nasServerID, err.Error()) + } + c.nasName = nas.Name volumeResponse := getCSIVolume(alreadyExistVolume.ID, sizeInBytes) return volumeResponse, nil } diff --git a/pkg/controller/creator_test.go b/pkg/controller/creator_test.go index f380da92..854d1e33 100644 --- a/pkg/controller/creator_test.go +++ b/pkg/controller/creator_test.go @@ -111,10 +111,12 @@ func TestVolumeCreator_CheckIfAlreadyExists(t *testing.T) { nc := &NfsCreator{} name := "test" sizeInBytes := int64(1610612736) + validNodeID = strings.Join([]string{validHostName, "127.0.0.1"}, "-") clientMock := new(mocks.Client) clientMock.On("GetFSByName", context.Background(), name). Return(gopowerstore.FileSystem{SizeTotal: 3221225472}, nil) + clientMock.On("GetNAS", context.Background(), mock.Anything).Return(gopowerstore.NAS{CurrentNodeID: validNodeID}, nil) vol, err := nc.CheckIfAlreadyExists(context.Background(), name, sizeInBytes, clientMock) assert.NoError(t, err) assert.Equal(t, sizeInBytes, vol.CapacityBytes) From 367f666ba97aec23944af94a1b25b8d68cb554a3 Mon Sep 17 00:00:00 2001 From: francis-nijay Date: Mon, 3 Nov 2025 06:40:39 -0500 Subject: [PATCH 2/5] Release v2.15.1 changes --- go.mod | 2 +- go.sum | 4 ++-- pkg/controller/controller.go | 2 -- pkg/controller/creator.go | 1 - 4 files changed, 3 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index 415626fa..60729be2 100644 --- a/go.mod +++ b/go.mod @@ -17,7 +17,7 @@ require ( github.com/dell/gofsutil v1.20.0 github.com/dell/goiscsi v1.13.0 github.com/dell/gonvme v1.12.0 - github.com/dell/gopowerstore v1.20.0 + github.com/dell/gopowerstore v1.20.1 github.com/fsnotify/fsnotify v1.9.0 github.com/go-openapi/strfmt v0.23.0 github.com/golang/mock v1.6.0 diff --git a/go.sum b/go.sum index 4c3b4375..8c593b60 100644 --- a/go.sum +++ b/go.sum @@ -121,8 +121,8 @@ github.com/dell/goiscsi v1.13.0 h1:4+uB+uJQmJ91yN7wy38sLsr5S/lqL3/tVboLOh0sg38= github.com/dell/goiscsi v1.13.0/go.mod h1:1IPCAavfm6T9BzKS0QYfBlJz7X+AfYPYjH4G84TvJP4= github.com/dell/gonvme v1.12.0 h1:KLOr+v+1kn/sz26CFTAkFrR1Ti4aZ37i1Mlxp1hBXYs= github.com/dell/gonvme v1.12.0/go.mod h1:ETLwyr+OG3DYfzdlMKCv5PjeDfj+JIxV2xrbHBTg2lk= -github.com/dell/gopowerstore v1.20.0 h1:hGnSahY4/om48xFpD9dQFvHfDjFN62n+y8UlOEUAii8= -github.com/dell/gopowerstore v1.20.0/go.mod h1:PNkGw7gZUyyjdJdZdKKvGve30DMse/SzOQqUkVN8HCM= +github.com/dell/gopowerstore v1.20.1 h1:Z2N5eWVBG+SrwtPMEXVeGOJ3jquhkrZ6PyCFpwfkDSg= +github.com/dell/gopowerstore v1.20.1/go.mod h1:PNkGw7gZUyyjdJdZdKKvGve30DMse/SzOQqUkVN8HCM= github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ= github.com/dgryski/go-sip13 v0.0.0-20181026042036-e10d5fee7954/go.mod h1:vAd38F8PWV+bWy6jNmig1y/TA+kYO4g3RSRF0IAv0no= github.com/docopt/docopt-go v0.0.0-20180111231733-ee0de3bc6815/go.mod h1:WwZ+bS3ebgob9U8Nd0kOddGdZWjyMGR8Wziv+TBNwSE= diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 80170fac..fa570f97 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -441,8 +441,6 @@ func (s *Service) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest var volumeResponse *csi.Volume - - // check if job is already in progress on array, if so, return error and let CO check again if useNFS { jobs, err := arr.Client.GetInProgressJobsByFsName(ctx, req.GetName()) diff --git a/pkg/controller/creator.go b/pkg/controller/creator.go index deca43c0..08f3df5e 100644 --- a/pkg/controller/creator.go +++ b/pkg/controller/creator.go @@ -421,7 +421,6 @@ func (c *NfsCreator) CheckIfAlreadyExists(ctx context.Context, name string, size } log.Infof("filesystem '%s' already exists", name) - // update the nas server name for the volume to ensure CreateVolume adds the correct nas to volume context nasServerID := alreadyExistVolume.NasServerID nas, err := client.GetNAS(ctx, nasServerID) From 72a13ee9252abc33b07a6139fe21990fd0f8c7f8 Mon Sep 17 00:00:00 2001 From: francis-nijay Date: Fri, 7 Nov 2025 01:46:13 -0500 Subject: [PATCH 3/5] Fix lint error --- pkg/controller/creator_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/controller/creator_test.go b/pkg/controller/creator_test.go index 854d1e33..8b9d731d 100644 --- a/pkg/controller/creator_test.go +++ b/pkg/controller/creator_test.go @@ -22,6 +22,8 @@ import ( "context" "errors" "testing" + "strings" + "github.com/container-storage-interface/spec/lib/go/csi" "github.com/dell/gopowerstore" From 345d88aa0dacc329fb88abfa5e0e3ff2e50e5584 Mon Sep 17 00:00:00 2001 From: francis-nijay Date: Fri, 7 Nov 2025 01:52:30 -0500 Subject: [PATCH 4/5] Fix lint error --- pkg/controller/controller_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index 5140c851..57b21cb5 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -28,7 +28,6 @@ import ( "testing" "time" - "github.com/dell/csm-sharednfs/nfs" csiext "github.com/dell/dell-csi-extensions/replication" "github.com/dell/csi-powerstore/v2/mocks" From f7c63558eabde35f43d2b27c4b72414d3b97a507 Mon Sep 17 00:00:00 2001 From: francis-nijay Date: Fri, 7 Nov 2025 01:58:09 -0500 Subject: [PATCH 5/5] Fix lint error --- pkg/controller/creator_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/controller/creator_test.go b/pkg/controller/creator_test.go index 8b9d731d..730b8e98 100644 --- a/pkg/controller/creator_test.go +++ b/pkg/controller/creator_test.go @@ -21,9 +21,8 @@ package controller import ( "context" "errors" - "testing" "strings" - + "testing" "github.com/container-storage-interface/spec/lib/go/csi" "github.com/dell/gopowerstore"