From a6ed343daa8e5e9eaa7fab0f36b47a79725aeb59 Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Mon, 21 Oct 2024 11:02:07 -0400 Subject: [PATCH 1/2] user must pause metro session to expand metro volume --- pkg/controller/controller.go | 52 ++++++-------------- pkg/controller/controller_test.go | 81 +++---------------------------- 2 files changed, 24 insertions(+), 109 deletions(-) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 3832574d..3a402b78 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -1340,63 +1340,42 @@ func (s *Service) ListSnapshots(ctx context.Context, req *csi.ListSnapshotsReque }, nil } -func pauseMetroSession(ctx context.Context, metroSessionID string, arr *array.PowerStoreArray) (paused bool, err error) { +func isPausedMetroSession(ctx context.Context, metroSessionID string, arr *array.PowerStoreArray) (paused bool, err error) { metroSession, err := arr.Client.GetReplicationSessionByID(ctx, metroSessionID) if err != nil { return false, fmt.Errorf("could not get metro replication session %s", metroSessionID) } - // confirm the session is in a state we can pause from. - if metroSession.State != gopowerstore.RsStateOk && - metroSession.State != gopowerstore.RsStateSynchronizing && - metroSession.State != gopowerstore.RsStatePaused && - metroSession.State != gopowerstore.RsStateSystemPaused && - metroSession.State != gopowerstore.RsStateFractured { - return false, fmt.Errorf("could not pause the metro replication session, %s, because the session is not in expected state to pause", metroSession.ID) - } + log.Debugf("checking if metro replication session, %s, is paused", metroSession.ID) if metroSession.State != gopowerstore.RsStatePaused { - log.Debugf("pausing metro replication session, %s", metroSession.ID) - - // pause the replication session - _, err := arr.Client.ExecuteActionOnReplicationSession(ctx, metroSession.ID, gopowerstore.RsActionPause, nil) - if err != nil { - return false, fmt.Errorf("metro replication session, %s, could not be paused: %s", metroSession.ID, err.Error()) - } - } else { - log.Debugf("metro replication session, %s, already paused", metroSession.ID) + return false, errors.New("metro replication session not in 'paused' state") } + + log.Debugf("metro replication session, %s, is paused", metroSession.ID) + return true, nil } -func resumeMetroSession(ctx context.Context, metroSessionID string, array *array.PowerStoreArray) (resumed bool, err error) { +func isResumedMetroSession(ctx context.Context, metroSessionID string, array *array.PowerStoreArray) (resumed bool, err error) { metroSession, err := array.Client.GetReplicationSessionByID(ctx, metroSessionID) if err != nil { return false, fmt.Errorf("could not get metro replication session: %s", err.Error()) } + log.Debugf("checking if metro replication session, %s, has been resumed", metroSession.ID) + // nothing to do if not paused if metroSession.State == gopowerstore.RsStateOk || metroSession.State == gopowerstore.RsStateSynchronizing || metroSession.State == gopowerstore.RsStateResuming || metroSession.State == gopowerstore.RsStateSwitchingToMetroSync || metroSession.State == gopowerstore.RsStateFractured { - log.Debugf("metro replication session, %s, already resumed", metroSession.ID) - return false, nil + log.Debugf("metro replication session, %s, has been resumed", metroSession.ID) + return true, nil } - // metro session can only be resumed if it is in 'paused' state - if metroSession.State != gopowerstore.RsStatePaused { - return false, errors.New("the metro session must be in 'paused' state before resuming") - } - - log.Debugf("resuming metro replication session %s", metroSession.ID) - _, err = array.Client.ExecuteActionOnReplicationSession(ctx, metroSession.ID, gopowerstore.RsActionResume, nil) - if err != nil { - return false, fmt.Errorf("metro replication session, %s, could not be resumed: %s", metroSession.ID, err.Error()) - } - - return true, nil + return false, errors.New("the metro replication session has not been resumed") } // ControllerExpandVolume resizes Volume or FileSystem by increasing available volume capacity in the storage array. @@ -1433,10 +1412,11 @@ func (s *Service) ControllerExpandVolume(ctx context.Context, req *csi.Controlle if vol.Size < requiredBytes { if isMetro { // must pause metro session before modifying the volume - _, err = pauseMetroSession(ctx, vol.MetroReplicationSessionID, array) + _, err = isPausedMetroSession(ctx, vol.MetroReplicationSessionID, array) if err != nil { return nil, status.Errorf(codes.Internal, - "failed to expand the volume %s because the metro replication session could not be paused: %s", vol.Name, err.Error()) + "failed to expand the volume, %s, because the metro replication session is not paused. please pause the metro replication session: %s", + vol.Name, err.Error()) } } @@ -1450,7 +1430,7 @@ func (s *Service) ControllerExpandVolume(ctx context.Context, req *csi.Controlle // check the metro session state and resume if necessary // in case the previous request failed after expanding the volume, resume the session if isMetro { - volExpanded, err = resumeMetroSession(ctx, vol.MetroReplicationSessionID, array) + volExpanded, err = isResumedMetroSession(ctx, vol.MetroReplicationSessionID, array) if err != nil { return nil, status.Errorf(codes.Internal, "failed to expand the volume %s because the metro replication session could not be resumed: %s", vol.Name, err.Error()) diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index a3b93a5b..de033e6d 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -2295,17 +2295,13 @@ var _ = ginkgo.Describe("CSIControllerService", func() { // Return okay to pause session clientMock.On("GetReplicationSessionByID", mock.Anything, validSessionID).Return(gopowerstore.ReplicationSession{ ID: validSessionID, - State: gopowerstore.RsStateOk, + State: gopowerstore.RsStatePaused, }, nil).Times(1) // Return paused to resume session clientMock.On("GetReplicationSessionByID", mock.Anything, validSessionID).Return(gopowerstore.ReplicationSession{ ID: validSessionID, - State: gopowerstore.RsStatePaused, + State: gopowerstore.RsStateOk, }, nil).Times(1) - clientMock.On("ExecuteActionOnReplicationSession", mock.Anything, validSessionID, gopowerstore.RsActionPause, (*gopowerstore.FailoverParams)(nil)). - Return(gopowerstore.EmptyResponse(""), nil) - clientMock.On("ExecuteActionOnReplicationSession", mock.Anything, validSessionID, gopowerstore.RsActionResume, (*gopowerstore.FailoverParams)(nil)). - Return(gopowerstore.EmptyResponse(""), nil) req := getTypicalControllerExpandRequest(validMetroBlockVolumeID, validVolSize*2) res, err := ctrlSvc.ControllerExpandVolume(context.Background(), req) @@ -2381,34 +2377,25 @@ var _ = ginkgo.Describe("CSIControllerService", func() { gomega.Expect(err.Error()).To(gomega.ContainSubstring("could not get metro replication session")) }) - ginkgo.It("should fail when trying to pause metro session due to API error", func() { - e := errors.New("some-api-error") + ginkgo.It("should fail if metro session is not paused", func() { clientMock.On("GetVolume", mock.Anything, validBaseVolID).Return(gopowerstore.Volume{ MetroReplicationSessionID: validSessionID, Size: validVolSize, }, nil) - clientMock.On("ModifyVolume", - mock.Anything, - mock.AnythingOfType("*gopowerstore.VolumeModify"), - validBaseVolID). - Return(gopowerstore.EmptyResponse(""), nil) - // Return okay to pause session + // Return error state for pause failure clientMock.On("GetReplicationSessionByID", mock.Anything, validSessionID).Return(gopowerstore.ReplicationSession{ ID: validSessionID, State: gopowerstore.RsStateOk, }, nil).Times(1) - clientMock.On("ExecuteActionOnReplicationSession", mock.Anything, validSessionID, gopowerstore.RsActionPause, (*gopowerstore.FailoverParams)(nil)). - Return(gopowerstore.EmptyResponse(""), e) req := getTypicalControllerExpandRequest(validMetroBlockVolumeID, validVolSize*2) _, err := ctrlSvc.ControllerExpandVolume(context.Background(), req) gomega.Expect(err).ToNot(gomega.BeNil()) - gomega.Expect(err.Error()).To(gomega.ContainSubstring("because the metro replication session could not be paused")) + gomega.Expect(err.Error()).To(gomega.ContainSubstring("metro replication session not in 'paused' state")) }) - ginkgo.It("should fail when trying to resume metro session due to API error", func() { - e := errors.New("some-api-error") + ginkgo.It("should fail if metro session is not resumed after volume expansion", func() { clientMock.On("GetVolume", mock.Anything, validBaseVolID).Return(gopowerstore.Volume{ MetroReplicationSessionID: validSessionID, Size: validVolSize, @@ -2419,73 +2406,21 @@ var _ = ginkgo.Describe("CSIControllerService", func() { validBaseVolID). Return(gopowerstore.EmptyResponse(""), nil) // Return okay to pause session - clientMock.On("GetReplicationSessionByID", mock.Anything, validSessionID).Return(gopowerstore.ReplicationSession{ - ID: validSessionID, - State: gopowerstore.RsStateOk, - }, nil).Times(1) - // Return paused to resume session clientMock.On("GetReplicationSessionByID", mock.Anything, validSessionID).Return(gopowerstore.ReplicationSession{ ID: validSessionID, State: gopowerstore.RsStatePaused, }, nil).Times(1) - clientMock.On("ExecuteActionOnReplicationSession", mock.Anything, validSessionID, gopowerstore.RsActionPause, (*gopowerstore.FailoverParams)(nil)). - Return(gopowerstore.EmptyResponse(""), nil) - clientMock.On("ExecuteActionOnReplicationSession", mock.Anything, validSessionID, gopowerstore.RsActionResume, (*gopowerstore.FailoverParams)(nil)). - Return(gopowerstore.EmptyResponse(""), e) - - req := getTypicalControllerExpandRequest(validMetroBlockVolumeID, validVolSize*2) - _, err := ctrlSvc.ControllerExpandVolume(context.Background(), req) - - gomega.Expect(err).ToNot(gomega.BeNil()) - gomega.Expect(err.Error()).To(gomega.ContainSubstring("because the metro replication session could not be resumed")) - }) - - ginkgo.It("should fail when trying to pause metro session due to unexpected state", func() { - clientMock.On("GetVolume", mock.Anything, validBaseVolID).Return(gopowerstore.Volume{ - MetroReplicationSessionID: validSessionID, - Size: validVolSize, - }, nil) - // Return error state for pause failure - clientMock.On("GetReplicationSessionByID", mock.Anything, validSessionID).Return(gopowerstore.ReplicationSession{ - ID: validSessionID, - State: gopowerstore.RsStateError, - }, nil).Times(1) - - req := getTypicalControllerExpandRequest(validMetroBlockVolumeID, validVolSize*2) - _, err := ctrlSvc.ControllerExpandVolume(context.Background(), req) - - gomega.Expect(err).ToNot(gomega.BeNil()) - gomega.Expect(err.Error()).To(gomega.ContainSubstring("because the session is not in expected state to pause")) - }) - - ginkgo.It("should fail when trying to resume metro session due to unexpected state", func() { - clientMock.On("GetVolume", mock.Anything, validBaseVolID).Return(gopowerstore.Volume{ - MetroReplicationSessionID: validSessionID, - Size: validVolSize, - }, nil) - clientMock.On("ModifyVolume", - mock.Anything, - mock.AnythingOfType("*gopowerstore.VolumeModify"), - validBaseVolID). - Return(gopowerstore.EmptyResponse(""), nil) - // Return okay to pause session - clientMock.On("GetReplicationSessionByID", mock.Anything, validSessionID).Return(gopowerstore.ReplicationSession{ - ID: validSessionID, - State: gopowerstore.RsStateOk, - }, nil).Times(1) // Return error state for resume failure clientMock.On("GetReplicationSessionByID", mock.Anything, validSessionID).Return(gopowerstore.ReplicationSession{ ID: validSessionID, - State: gopowerstore.RsStateError, + State: gopowerstore.RsStatePaused, }, nil).Times(1) - clientMock.On("ExecuteActionOnReplicationSession", mock.Anything, validSessionID, gopowerstore.RsActionPause, (*gopowerstore.FailoverParams)(nil)). - Return(gopowerstore.EmptyResponse(""), nil) req := getTypicalControllerExpandRequest(validMetroBlockVolumeID, validVolSize*2) _, err := ctrlSvc.ControllerExpandVolume(context.Background(), req) gomega.Expect(err).ToNot(gomega.BeNil()) - gomega.Expect(err.Error()).To(gomega.ContainSubstring("the metro session must be in 'paused' state before resuming")) + gomega.Expect(err.Error()).To(gomega.ContainSubstring("the metro replication session has not been resumed")) }) }) From 8345a149d99db0ddb2db9995600eae4e60f8f48b Mon Sep 17 00:00:00 2001 From: "Lau, Luke" Date: Mon, 21 Oct 2024 12:53:05 -0400 Subject: [PATCH 2/2] remove metro session "resume" check --- pkg/controller/controller.go | 36 +------------------------------ pkg/controller/controller_test.go | 35 +----------------------------- 2 files changed, 2 insertions(+), 69 deletions(-) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 3a402b78..babf482a 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -1357,27 +1357,6 @@ func isPausedMetroSession(ctx context.Context, metroSessionID string, arr *array return true, nil } -func isResumedMetroSession(ctx context.Context, metroSessionID string, array *array.PowerStoreArray) (resumed bool, err error) { - metroSession, err := array.Client.GetReplicationSessionByID(ctx, metroSessionID) - if err != nil { - return false, fmt.Errorf("could not get metro replication session: %s", err.Error()) - } - - log.Debugf("checking if metro replication session, %s, has been resumed", metroSession.ID) - - // nothing to do if not paused - if metroSession.State == gopowerstore.RsStateOk || - metroSession.State == gopowerstore.RsStateSynchronizing || - metroSession.State == gopowerstore.RsStateResuming || - metroSession.State == gopowerstore.RsStateSwitchingToMetroSync || - metroSession.State == gopowerstore.RsStateFractured { - log.Debugf("metro replication session, %s, has been resumed", metroSession.ID) - return true, nil - } - - return false, errors.New("the metro replication session has not been resumed") -} - // ControllerExpandVolume resizes Volume or FileSystem by increasing available volume capacity in the storage array. func (s *Service) ControllerExpandVolume(ctx context.Context, req *csi.ControllerExpandVolumeRequest) (*csi.ControllerExpandVolumeResponse, error) { id, arrayID, protocol, remoteVolumeID, _, err := array.ParseVolumeID(ctx, req.VolumeId, s.DefaultArray(), nil) @@ -1402,7 +1381,6 @@ func (s *Service) ControllerExpandVolume(ctx context.Context, req *csi.Controlle return nil, status.Error(codes.NotFound, "detected SCSI protocol but wasn't able to fetch the volume info") } - volExpanded := false // to return appropriate response based on whether the volume is expanded or not isMetro := remoteVolumeID != "" if isMetro && vol.MetroReplicationSessionID == "" { return nil, status.Errorf(codes.Internal, @@ -1424,21 +1402,9 @@ func (s *Service) ControllerExpandVolume(ctx context.Context, req *csi.Controlle if err != nil { return nil, status.Errorf(codes.Internal, "unable to modify volume size: %s", err.Error()) } - volExpanded = true - } - - // check the metro session state and resume if necessary - // in case the previous request failed after expanding the volume, resume the session - if isMetro { - volExpanded, err = isResumedMetroSession(ctx, vol.MetroReplicationSessionID, array) - if err != nil { - return nil, status.Errorf(codes.Internal, - "failed to expand the volume %s because the metro replication session could not be resumed: %s", vol.Name, err.Error()) - } - } - if volExpanded { return &csi.ControllerExpandVolumeResponse{CapacityBytes: requiredBytes, NodeExpansionRequired: true}, nil } + return &csi.ControllerExpandVolumeResponse{}, nil } diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index de033e6d..ed4e7231 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -2292,16 +2292,11 @@ var _ = ginkgo.Describe("CSIControllerService", func() { mock.AnythingOfType("*gopowerstore.VolumeModify"), validBaseVolID). Return(gopowerstore.EmptyResponse(""), nil) - // Return okay to pause session + // Return metro session status as paused clientMock.On("GetReplicationSessionByID", mock.Anything, validSessionID).Return(gopowerstore.ReplicationSession{ ID: validSessionID, State: gopowerstore.RsStatePaused, }, nil).Times(1) - // Return paused to resume session - clientMock.On("GetReplicationSessionByID", mock.Anything, validSessionID).Return(gopowerstore.ReplicationSession{ - ID: validSessionID, - State: gopowerstore.RsStateOk, - }, nil).Times(1) req := getTypicalControllerExpandRequest(validMetroBlockVolumeID, validVolSize*2) res, err := ctrlSvc.ControllerExpandVolume(context.Background(), req) @@ -2394,34 +2389,6 @@ var _ = ginkgo.Describe("CSIControllerService", func() { gomega.Expect(err).ToNot(gomega.BeNil()) gomega.Expect(err.Error()).To(gomega.ContainSubstring("metro replication session not in 'paused' state")) }) - - ginkgo.It("should fail if metro session is not resumed after volume expansion", func() { - clientMock.On("GetVolume", mock.Anything, validBaseVolID).Return(gopowerstore.Volume{ - MetroReplicationSessionID: validSessionID, - Size: validVolSize, - }, nil) - clientMock.On("ModifyVolume", - mock.Anything, - mock.AnythingOfType("*gopowerstore.VolumeModify"), - validBaseVolID). - Return(gopowerstore.EmptyResponse(""), nil) - // Return okay to pause session - clientMock.On("GetReplicationSessionByID", mock.Anything, validSessionID).Return(gopowerstore.ReplicationSession{ - ID: validSessionID, - State: gopowerstore.RsStatePaused, - }, nil).Times(1) - // Return error state for resume failure - clientMock.On("GetReplicationSessionByID", mock.Anything, validSessionID).Return(gopowerstore.ReplicationSession{ - ID: validSessionID, - State: gopowerstore.RsStatePaused, - }, nil).Times(1) - - req := getTypicalControllerExpandRequest(validMetroBlockVolumeID, validVolSize*2) - _, err := ctrlSvc.ControllerExpandVolume(context.Background(), req) - - gomega.Expect(err).ToNot(gomega.BeNil()) - gomega.Expect(err.Error()).To(gomega.ContainSubstring("the metro replication session has not been resumed")) - }) }) ginkgo.When("expanding nfs volume", func() {