-
Notifications
You must be signed in to change notification settings - Fork 89
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PWX-32230: Supporting unidirectional clusterpair creation with storkctl create command. #1455
Conversation
Can one of the admins verify this patch? |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #1455 +/- ##
==========================================
+ Coverage 66.05% 66.68% +0.62%
==========================================
Files 43 43
Lines 4984 4991 +7
==========================================
+ Hits 3292 3328 +36
+ Misses 1373 1339 -34
- Partials 319 324 +5
☔ View full report in Codecov by Sentry. |
pkg/storkctl/clusterpair.go
Outdated
@@ -598,7 +606,7 @@ func newCreateClusterPairCommand(cmdFactory Factory, ioStreams genericclioptions | |||
createClusterPairCommand.Flags().StringVarP(&destToken, "dest-token", "", "", "(Optional)Destination cluster token for cluster pairing") | |||
createClusterPairCommand.Flags().StringVarP(&projectMappingsStr, "project-mappings", "", "", projectMappingHelpString) | |||
createClusterPairCommand.Flags().StringVarP(&mode, "mode", "", "async-dr", "Mode of DR. [async-dr, sync-dr]") | |||
|
|||
createClusterPairCommand.Flags().BoolVarP(&uniDirectional, "uni-directional", "u", false, "Required to create Clusterpair from source -> dest only") |
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.
Instead of "Required to", I think we can mention (Optional) To
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.
Done
pkg/storkctl/clusterpair.go
Outdated
@@ -430,7 +434,7 @@ func newCreateClusterPairCommand(cmdFactory Factory, ioStreams genericclioptions | |||
credentialData["encryptionKey"] = []byte(encryptionKey) | |||
|
|||
// Bail out if portworx-api service type is not loadbalancer type and the endpoints are not provided. | |||
if len(sEP) == 0 { | |||
if !uniDirectional && len(sEP) == 0 { |
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.
I am trying to understand change in this line. Can you please explain?
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.
We don't have to figure out portworx endpoint in source cluster for unidirectional clusterpair.
Also with this check the output of unidirectional create command does not print the following kind of line
Source portworx endpoint is 10.10.101.100:9001
UTs for this package already exists. We should update the UTs to account for the code coverage. |
d91c52b
to
187ee4a
Compare
Added. |
pkg/storkctl/clusterpair.go
Outdated
@@ -253,6 +253,7 @@ func newCreateClusterPairCommand(cmdFactory Factory, ioStreams genericclioptions | |||
var syncDR bool | |||
var pxAuthTokenSrc, pxAuthSecretNamespaceSrc string | |||
var pxAuthTokenDest, pxAuthSecretNamespaceDest string | |||
var uniDirectional bool |
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.
nit..unidirectional
Looks good. |
…tl create command.
Signed-Off-By: Diptiranjan
What type of PR is this?
improvement
What this PR does / why we need it:
With storkctl create option, we can create unidirectional cluster pair.
Does this PR change a user-facing CRD or CLI?:
Is a release note needed?:
Does this change need to be cherry-picked to a release branch?:
Test
Test result has been updated in PWX-32230.