Skip to content

Commit

Permalink
Addressed the review comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
@diptiranjanpx
diptiranjanpx committed Sep 13, 2023
1 parent 3133788 commit fa1f860
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 37 deletions.
61 changes: 31 additions & 30 deletions pkg/storkctl/clusterpair.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -298,14 +298,17 @@ func newCreateClusterPairCommand(cmdFactory Factory, ioStreams genericclioptions

if sFile == dFile {
util.CheckErr(fmt.Errorf("source kubeconfig file and destination kubeconfig file should be different"))
return
}

sameFiles, err := utils.CompareFiles(sFile, dFile)
if err != nil {
util.CheckErr(err)
return

Check warning on line 307 in pkg/storkctl/clusterpair.go

View check run for this annotation

Codecov / codecov/patch

pkg/storkctl/clusterpair.go#L307 

Added line #L307 was not covered by tests
}
if sameFiles {
util.CheckErr(fmt.Errorf("source kubeconfig and destination kubeconfig file should be different"))
return
}
// Handling the syncDR cases here
if syncDR {
Expand Down Expand Up @@ -371,20 +374,6 @@ func newCreateClusterPairCommand(cmdFactory Factory, ioStreams genericclioptions
}

//Handling the asyncDR cases here onwards

if len(cmdFactory.GetNamespace()) > 0 {
err = createNamespace(sFile, cmdFactory.GetNamespace())
if err != nil {
util.CheckErr(fmt.Errorf("error in creating namespace %s in source cluster: %v", cmdFactory.GetNamespace(), err))
return
}
err = createNamespace(dFile, cmdFactory.GetNamespace())
if err != nil {
util.CheckErr(fmt.Errorf("error in creating namespace %s in destination cluster: %v", cmdFactory.GetNamespace(), err))
return
}
}

// Bail out if portworx-api service type is not loadbalancer type and the endpoints are not provided.
if !unidirectional && len(sEP) == 0 {
srcPXEndpoint, err := getPXEndPointDetails(sFile)
Expand Down Expand Up @@ -427,18 +416,30 @@ func newCreateClusterPairCommand(cmdFactory Factory, ioStreams genericclioptions
destToken = token
}

sconf, err := getConfig(sFile).ClientConfig()
sConf, err := getConfig(sFile).ClientConfig()

Check warning on line 419 in pkg/storkctl/clusterpair.go

View check run for this annotation

Codecov / codecov/patch

pkg/storkctl/clusterpair.go#L419 

Added line #L419 was not covered by tests
if err != nil {
util.CheckErr(err)
return
}
dconf, err := getConfig(dFile).ClientConfig()
dConf, err := getConfig(dFile).ClientConfig()

Check warning on line 424 in pkg/storkctl/clusterpair.go

View check run for this annotation

Codecov / codecov/patch

pkg/storkctl/clusterpair.go#L424 

Added line #L424 was not covered by tests
if err != nil {
util.CheckErr(err)
return
}

finalBackupLocationName := backupLocationName
if len(cmdFactory.GetNamespace()) > 0 {
err = createNamespace(sFile, cmdFactory.GetNamespace())
if err != nil {
util.CheckErr(fmt.Errorf("error in creating namespace %s in source cluster: %v", cmdFactory.GetNamespace(), err))
return

Check warning on line 434 in pkg/storkctl/clusterpair.go

View check run for this annotation

Codecov / codecov/patch

pkg/storkctl/clusterpair.go#L433-L434 

Added lines #L433 - L434 were not covered by tests
}
err = createNamespace(dFile, cmdFactory.GetNamespace())
if err != nil {
util.CheckErr(fmt.Errorf("error in creating namespace %s in destination cluster: %v", cmdFactory.GetNamespace(), err))

Check warning on line 438 in pkg/storkctl/clusterpair.go

View check run for this annotation

Codecov / codecov/patch

pkg/storkctl/clusterpair.go#L436-L438 

Added lines #L436 - L438 were not covered by tests
return
}
}

// backuplocation object init as it is used in different if blocks
backupLocation := &storkv1.BackupLocation{

Check warning on line 444 in pkg/storkctl/clusterpair.go

View check run for this annotation

Codecov / codecov/patch

pkg/storkctl/clusterpair.go#L444 

Added line #L444 was not covered by tests
ObjectMeta: meta.ObjectMeta{
Expand All @@ -455,12 +456,12 @@ func newCreateClusterPairCommand(cmdFactory Factory, ioStreams genericclioptions
if len(backupLocationName) > 0 {
err = verifyBackupLocationExistence(sFile, backupLocationName, cmdFactory.GetNamespace())
if err != nil {
util.CheckErr(fmt.Errorf("input backuplocation %s existence check failed with error in source cluster: %v", backupLocationName, err))
util.CheckErr(fmt.Errorf("input objectstorelocation %s existence check failed with error in source cluster: %v", backupLocationName, err))
return

Check warning on line 460 in pkg/storkctl/clusterpair.go

View check run for this annotation

Codecov / codecov/patch

pkg/storkctl/clusterpair.go#L456-L460 

Added lines #L456 - L460 were not covered by tests
}
err = verifyBackupLocationExistence(dFile, backupLocationName, cmdFactory.GetNamespace())
if err != nil {
util.CheckErr(fmt.Errorf("input backuplocation %s existence check failed with error in destination cluster: %v", backupLocationName, err))
util.CheckErr(fmt.Errorf("input objectstorelocation %s existence check failed with error in destination cluster: %v", backupLocationName, err))
return

Check warning on line 465 in pkg/storkctl/clusterpair.go

View check run for this annotation

Codecov / codecov/patch

pkg/storkctl/clusterpair.go#L462-L465 

Added lines #L462 - L465 were not covered by tests
}
} else {

Check warning on line 467 in pkg/storkctl/clusterpair.go

View check run for this annotation

Codecov / codecov/patch

pkg/storkctl/clusterpair.go#L467 

Added line #L467 was not covered by tests
Expand Down Expand Up @@ -528,11 +529,11 @@ func newCreateClusterPairCommand(cmdFactory Factory, ioStreams genericclioptions
credentialData["path"] = []byte(bucket)
credentialData["type"] = []byte(provider)
credentialData["encryptionKey"] = []byte(encryptionKey)
finalBackupLocationName = backupLocation.Name
backupLocationName = backupLocation.Name

Check warning on line 532 in pkg/storkctl/clusterpair.go

View check run for this annotation

Codecov / codecov/patch

pkg/storkctl/clusterpair.go#L529-L532 

Added lines #L529 - L532 were not covered by tests

printMsg(fmt.Sprintf("\nCreating Secret and ObjectstoreLocation in source cluster in namespace %v...", cmdFactory.GetNamespace()), ioStreams.Out)
storkops.Instance().SetConfig(sconf)
core.Instance().SetConfig(sconf)
storkops.Instance().SetConfig(sConf)
core.Instance().SetConfig(sConf)
secret := &v1.Secret{
ObjectMeta: meta.ObjectMeta{
Name: clusterPairName,
Expand Down Expand Up @@ -562,8 +563,8 @@ func newCreateClusterPairCommand(cmdFactory Factory, ioStreams genericclioptions
}
printMsg(fmt.Sprintf("ObjectstoreLocation %v created on source cluster in namespace %v\n", clusterPairName, cmdFactory.GetNamespace()), ioStreams.Out)

Check warning on line 564 in pkg/storkctl/clusterpair.go

View check run for this annotation

Codecov / codecov/patch

pkg/storkctl/clusterpair.go#L564 

Added line #L564 was not covered by tests

storkops.Instance().SetConfig(dconf)
core.Instance().SetConfig(dconf)
storkops.Instance().SetConfig(dConf)
core.Instance().SetConfig(dConf)
printMsg(fmt.Sprintf("Creating Secret and ObjectstoreLocation in destination cluster in namespace %v...", cmdFactory.GetNamespace()), ioStreams.Out)
_, err = core.Instance().CreateSecret(secret)
if err != nil {
Expand All @@ -585,9 +586,9 @@ func newCreateClusterPairCommand(cmdFactory Factory, ioStreams genericclioptions
util.CheckErr(err)
return
}
storkops.Instance().SetConfig(sconf)
core.Instance().SetConfig(sconf)
srcClusterPair.Spec.Options[storkv1.BackupLocationResourceName] = finalBackupLocationName
storkops.Instance().SetConfig(sConf)
core.Instance().SetConfig(sConf)
srcClusterPair.Spec.Options[storkv1.BackupLocationResourceName] = backupLocationName
_, err = storkops.Instance().CreateClusterPair(srcClusterPair)

Check warning on line 592 in pkg/storkctl/clusterpair.go

View check run for this annotation

Codecov / codecov/patch

pkg/storkctl/clusterpair.go#L589-L592 

Added lines #L589 - L592 were not covered by tests
if err != nil {
util.CheckErr(err)
Expand Down Expand Up @@ -622,8 +623,8 @@ func newCreateClusterPairCommand(cmdFactory Factory, ioStreams genericclioptions
}

printMsg("Creating a cluster pair. Direction: Destination -> Source", ioStreams.Out)
storkops.Instance().SetConfig(dconf)
core.Instance().SetConfig(dconf)
storkops.Instance().SetConfig(dConf)
core.Instance().SetConfig(dConf)
destClusterPair.Spec.Options[storkv1.BackupLocationResourceName] = backupLocation.Name
_, err = storkops.Instance().CreateClusterPair(destClusterPair)
if err != nil {
Expand Down Expand Up @@ -652,7 +653,7 @@ func newCreateClusterPairCommand(cmdFactory Factory, ioStreams genericclioptions
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, "unidirectional", "u", false, "(Optional) to create Clusterpair from source -> dest only")
createClusterPairCommand.Flags().StringVarP(&backupLocationName, "use-existing-backuplocation", "", "", "(Optional) Backuplocation with the provided name should be present in both source and destination cluster")
createClusterPairCommand.Flags().StringVarP(&backupLocationName, "use-existing-objectstorelocation", "", "", "(Optional) Objectstorelocation with the provided name should be present in both source and destination cluster")
// New parameters for creating backuplocation secret
createClusterPairCommand.Flags().StringVarP(&provider, "provider", "p", "", "External objectstore provider name. [s3, azure, google]")
createClusterPairCommand.Flags().StringVar(&bucket, "bucket", "", "Bucket name")
Expand Down
12 changes: 5 additions & 7 deletions pkg/storkctl/clusterpair_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,24 +133,22 @@ func TestCreateUniDirectionalClusterPairMissingParameters(t *testing.T) {
expected := "error: missing parameter \"src-kube-file\" - Kubeconfig file missing for source cluster"
testCommon(t, cmdArgs, nil, expected, true)

srcConfig := createTempFile(t, "src.config", "source configuration")
destConfig := createTempFile(t, "dest.config", "destination configuration")
srcConfig := createTempFile(t, "src.config", "source")
destConfig := createTempFile(t, "dest.config", "destination")
defer os.Remove(srcConfig.Name())
defer os.Remove(destConfig.Name())

cmdArgs = []string{"create", "clusterpair", "uni-pair1", "-n", "test", "--src-kube-file", srcConfig.Name(), "--unidirectional"}
expected = "error: missing parameter \"dest-kube-file\" - Kubeconfig file missing for destination cluster"
testCommon(t, cmdArgs, nil, expected, true)

cmdArgs = []string{"create", "clusterpair", "uni-pair1", "-n", "test", "--src-kube-file", srcConfig.Name(), "--dest-kube-file", destConfig.Name(), "-u"}
expected = "error: missing parameter \"provider\" - External objectstore provider needs to be either of azure, google, s3"
testCommon(t, cmdArgs, nil, expected, true)

cmdArgs = []string{"create", "clusterpair", "uni-pair1", "-n", "test", "--src-kube-file", srcConfig.Name(), "--dest-kube-file", srcConfig.Name(), "-u"}
expected = "error: source kubeconfig file and destination kubeconfig file should be different"
testCommon(t, cmdArgs, nil, expected, true)

srcConfigDuplicate := createTempFile(t, "srcDup.config", "source configuration")
srcConfigDuplicate := createTempFile(t, "srcDup.config", "source")
defer os.Remove(srcConfigDuplicate.Name())

cmdArgs = []string{"create", "clusterpair", "uni-pair1", "-n", "test", "--src-kube-file", srcConfig.Name(), "--dest-kube-file", srcConfigDuplicate.Name(), "-u"}
expected = "error: source kubeconfig and destination kubeconfig file should be different"
testCommon(t, cmdArgs, nil, expected, true)
Expand Down

0 comments on commit fa1f860

Please sign in to comment.