Changes to support sync replication for Powerstore #348
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -263,27 +263,43 @@ func (s *Service) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest | |
| } | ||
| repMode := params[s.WithRP(KeyReplicationMode)] | ||
| if repMode == "" { | ||
| repMode = common.AsyncMode | ||
| } | ||
| repMode = strings.ToUpper(repMode) | ||
|
|
||
| switch repMode { | ||
| case common.SyncMode, common.AsyncMode: | ||
| // handle Sync and Async modes where protection policy with replication rule is applied on volume group | ||
| log.Infof("%s replication mode requested", repMode) | ||
| vgPrefix, ok := params[s.WithRP(KeyReplicationVGPrefix)] | ||
| if !ok { | ||
| return nil, status.Errorf(codes.InvalidArgument, "replication enabled but no volume group prefix specified in storage class") | ||
| } | ||
|
|
||
| rpo, ok := params[s.WithRP(KeyReplicationRPO)] | ||
| if !ok { | ||
| // If Replication mode is ASYNC and there is no RPO specified, returning an error | ||
| if repMode == common.AsyncMode { | ||
| return nil, status.Errorf(codes.InvalidArgument, "replication mode is ASYNC but no RPO specified in storage class") | ||
| } | ||
| // If Replication mode is SYNC and there is no RPO, defaulting the value to Zero | ||
| rpo = common.Zero | ||
| } | ||
| rpoEnum := gopowerstore.RPOEnum(rpo) | ||
| if err := rpoEnum.IsValid(); err != nil { | ||
| return nil, status.Errorf(codes.InvalidArgument, "invalid RPO value") | ||
| } | ||
|
|
||
| // Validating RPO to be non Zero when replication mode is ASYNC | ||
| if repMode == common.AsyncMode && rpo == common.Zero { | ||
| log.Errorf("RPO value for %s cannot be : %s", repMode, rpo) | ||
| return nil, status.Errorf(codes.InvalidArgument, "replication mode ASYNC requires RPO value to be non Zero") | ||
rajendraindukuri marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| // Validating RPO to be Zero whe replication mode is SYNC | ||
rajendraindukuri marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if repMode == common.SyncMode && rpo != common.Zero { | ||
| return nil, status.Errorf(codes.InvalidArgument, "replication mode SYNC requires RPO value to be Zero") | ||
| } | ||
| namespace := "" | ||
| if ignoreNS, ok := params[s.WithRP(KeyReplicationIgnoreNamespaces)]; ok && ignoreNS == "false" { | ||
| pvcNS, ok := params[KeyCSIPVCNamespace] | ||
|
|
@@ -302,15 +318,24 @@ func (s *Service) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest | |
| if apiError, ok := err.(gopowerstore.APIError); ok && apiError.NotFound() { | ||
| log.Infof("Volume group with name %s not found, creating it", vgName) | ||
|
|
||
| // Attribute that indicates whether snapshot sets of the volumegroup will be write-order consistent. | ||
| isWriteOrderConsistent := false | ||
|
There was a problem hiding this comment. This will make write-order-consistent to be false for ASYNC mode while the API/UI's enables it by default for volume group creation. There was a problem hiding this comment. How about enabling it (sending true) irrespective of the mode? There was a problem hiding this comment. Hi @santhoshatdell Currently for async we are not passing is-write-consistent ... so by default the value is going as false due to boolean default value (though from API perspective by default it is true ). For Sync, it should be true always so used this variable to pass as true.. Can we discuss on this and handle as part of upcoming PR s ? |
||
|
|
||
| // ensure protection policy exists | ||
| pp, err := EnsureProtectionPolicyExists(ctx, arr, vgName, remoteSystemName, rpoEnum) | ||
| if err != nil { | ||
| return nil, status.Errorf(codes.Internal, "can't ensure protection policy exists %s", err.Error()) | ||
| } | ||
|
|
||
| // To apply a ProtectionPolicy with Sync rule, VolumeGroup must be write-order-consistent | ||
| if repMode == common.SyncMode { | ||
| isWriteOrderConsistent = true | ||
| } | ||
|
|
||
| group, err := arr.Client.CreateVolumeGroup(ctx, &gopowerstore.VolumeGroupCreate{ | ||
| Name: vgName, | ||
| ProtectionPolicyID: pp, | ||
| IsWriteOrderConsistent: isWriteOrderConsistent, | ||
| }) | ||
| if err != nil { | ||
| return nil, status.Errorf(codes.Internal, "can't create volume group: %s", err.Error()) | ||
|
|
@@ -325,6 +350,12 @@ func (s *Service) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest | |
| return nil, status.Errorf(codes.Internal, "can't query volume group by name %s : %s", vgName, err.Error()) | ||
| } | ||
| } else { | ||
| // if Replication mode is SYNC, check if the VolumeGroup is write-order consistent | ||
| if repMode == common.SyncMode { | ||
| if !vg.IsWriteOrderConsistent { | ||
| return nil, status.Errorf(codes.Internal, "can't apply protection policy with sync rule if volume group is not write-order consistent") | ||
| } | ||
| } | ||
| // group exists, check that protection policy applied | ||
| if vg.ProtectionPolicyID == "" { | ||
| pp, err := EnsureProtectionPolicyExists(ctx, arr, vgName, remoteSystemName, rpoEnum) | ||
|
|
@@ -342,7 +373,7 @@ func (s *Service) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest | |
| if c, ok := creator.(*SCSICreator); ok { | ||
| c.vg = &vg | ||
| } | ||
| case common.Metro: | ||
| // handle Metro mode where metro is configured directly on the volume (or volume group if requested) | ||
| log.Info("Metro replication mode requested") | ||
|
|
||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'MetroMode' - to be consistent with other mode names? Or an enum that defines the modes.