Skip to content

Commit

Permalink
Revert "Convert required argument 'target plan' to '-p' flag."
Browse files Browse the repository at this point in the history
- This work never got finished, and it introduced a breaking change, so
we are reverting it for now.

This reverts commit d1cc3e7.

[#171027755]

Signed-off-by: James Palmer <[email protected]>
  • Loading branch information
pcf-core-services-writer committed Mar 24, 2020
1 parent e5ef114 commit e657c12
Show file tree
Hide file tree
Showing 8 changed files with 34 additions and 34 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ $ cf install-plugin -r CF-Community MysqlTools
Once the plugin is installed, migrate a v1 service instance to a new v2 service instance with the following command:

```
$ cf mysql-tools migrate V1-INSTANCE -p V2-PLAN
$ cf mysql-tools migrate V1-INSTANCE V2-PLAN
```

Where `V1-INSTANCE` is the the name of the v1 service instance that you wish to migrate, and `V2-PLAN` is the name of the v2 service plan to use for the new v2 service instance.
Expand Down
6 changes: 3 additions & 3 deletions main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@ import (
)

const (
migrateUsage = `cf mysql-tools migrate [-h] [--no-cleanup] [--skip-tls-validation] <source-service-instance> -p <p.mysql-plan-type>
migrateUsage = `cf mysql-tools migrate [-h] [--no-cleanup] [--skip-tls-validation] <source-service-instance> <p.mysql-plan-type>
`
findBindingUsage = `cf mysql-tools find-bindings [-h] <mysql-v1-service-name>
`
longUsage = `NAME:
mysql-tools - Plugin to migrate mysql instances
USAGE:
cf mysql-tools migrate [-h] [--no-cleanup] [--skip-tls-validation] <source-service-instance> -p <p.mysql-plan-type>
cf mysql-tools migrate [-h] [--no-cleanup] [--skip-tls-validation] <source-service-instance> <p.mysql-plan-type>
cf mysql-tools find-bindings [-h] <mysql-v1-service-name>
cf mysql-tools version
`
Expand Down Expand Up @@ -87,7 +87,7 @@ var _ = Describe("MysqlCliPlugin", func() {

Expect(string(session.Err.Contents())).To(Equal(
"Usage: " + migrateUsage +
"\nthe required flag `-p, --plan' was not specified\n"))
"\nthe required arguments `<source-service-instance>` and `<p.mysql-plan-type>` were not provided\n"))
})

It("find-binding requires exactly 1 arguments", func() {
Expand Down
18 changes: 9 additions & 9 deletions mysql-tools/plugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
"github.com/blang/semver"
"github.com/jessevdk/go-flags"
"github.com/pivotal-cf/mysql-cli-plugin/mysql-tools/cf"
"github.com/pivotal-cf/mysql-cli-plugin/mysql-tools/find-bindings"
find_bindings "github.com/pivotal-cf/mysql-cli-plugin/mysql-tools/find-bindings"
"github.com/pivotal-cf/mysql-cli-plugin/mysql-tools/migrate"
"github.com/pivotal-cf/mysql-cli-plugin/mysql-tools/presentation"
"github.com/pivotal-cf/mysql-cli-plugin/mysql-tools/unpack"
Expand All @@ -35,10 +35,10 @@ var (
)

const (
usage = `cf mysql-tools migrate [-h] [--no-cleanup] [--skip-tls-validation] <source-service-instance> -p <p.mysql-plan-type>
usage = `cf mysql-tools migrate [-h] [--no-cleanup] [--skip-tls-validation] <source-service-instance> <p.mysql-plan-type>
cf mysql-tools find-bindings [-h] <mysql-v1-service-name>
cf mysql-tools version`
migrateUsage = `cf mysql-tools migrate [-h] [--no-cleanup] [--skip-tls-validation] <source-service-instance> -p <p.mysql-plan-type>`
migrateUsage = `cf mysql-tools migrate [-h] [--no-cleanup] [--skip-tls-validation] <source-service-instance> <p.mysql-plan-type>`
findUsage = `cf mysql-tools find-bindings [-h] <mysql-v1-service-name>`
)

Expand Down Expand Up @@ -76,7 +76,7 @@ func (c *MySQLPlugin) Run(cliConnection plugin.CliConnection, args []string) {
mysql-tools - Plugin to migrate mysql instances
USAGE:
cf mysql-tools migrate [-h] [--no-cleanup] [--skip-tls-validation] <source-service-instance> -p <p.mysql-plan-type>
cf mysql-tools migrate [-h] [--no-cleanup] [--skip-tls-validation] <source-service-instance> <p.mysql-plan-type>
cf mysql-tools find-bindings [-h] <mysql-v1-service-name>
cf mysql-tools version`)
os.Exit(1)
Expand Down Expand Up @@ -155,11 +155,11 @@ func FindBindings(bf BindingFinder, args []string) error {
func Migrate(migrator Migrator, args []string) error {
var opts struct {
Args struct {
Source string `positional-arg-name:"<source-service-instance>"`
Source string `positional-arg-name:"<source-service-instance>"`
PlanName string `positional-arg-name:"<p.mysql-plan-type>"`
} `positional-args:"yes" required:"yes"`
NoCleanup bool `long:"no-cleanup" description:"don't clean up migration app and new service instance after a failed migration'"`
SkipTLSValidation bool `long:"skip-tls-validation" short:"k" description:"Skip certificate validation of the MySQL server certificate. Not recommended!"`
PlanName string `short:"p" long:"plan" description:"Service plan type" required:"yes"`
NoCleanup bool `long:"no-cleanup" description:"don't clean up migration app and new service instance after a failed migration"`
SkipTLSValidation bool `long:"skip-tls-validation" short:"k" description:"Skip certificate validation of the MySQL server certificate. Not recommended!"`
}

parser := flags.NewParser(&opts, flags.None)
Expand All @@ -175,7 +175,7 @@ func Migrate(migrator Migrator, args []string) error {
}
donorInstanceName := opts.Args.Source
tempRecipientInstanceName := donorInstanceName + "-new"
destPlan := opts.PlanName
destPlan := opts.Args.PlanName
cleanup := !opts.NoCleanup
skipTLSValidation := opts.SkipTLSValidation

Expand Down
22 changes: 11 additions & 11 deletions mysql-tools/plugin/plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ var _ = Describe("Plugin Commands", func() {
logOutput *bytes.Buffer
)

const migrateUsage = `Usage: cf mysql-tools migrate [-h] [--no-cleanup] [--skip-tls-validation] <source-service-instance> -p <p.mysql-plan-type>`
const migrateUsage = `Usage: cf mysql-tools migrate [-h] [--no-cleanup] [--skip-tls-validation] <source-service-instance> <p.mysql-plan-type>`
const findUsage = `Usage: cf mysql-tools find-bindings [-h] <mysql-v1-service-name>`

BeforeEach(func() {
Expand All @@ -48,7 +48,7 @@ var _ = Describe("Plugin Commands", func() {
Context("Migrate", func() {
It("migrates data from a source service instance to a newly created instance", func() {
args := []string{
"some-donor", "-p", "some-plan",
"some-donor", "some-plan",
}
Expect(plugin.Migrate(fakeMigrator, args)).To(Succeed())

Expand Down Expand Up @@ -97,7 +97,7 @@ var _ = Describe("Plugin Commands", func() {
It("Requests that the data be migrated insecurely", func() {
args := []string{
"--skip-tls-validation",
"some-donor", "-p", "some-plan",
"some-donor", "some-plan",
}
Expect(plugin.Migrate(fakeMigrator, args)).To(Succeed())

Expand All @@ -111,7 +111,7 @@ var _ = Describe("Plugin Commands", func() {
It("returns an error if the donor service instance does not exist", func() {
fakeMigrator.CheckServiceExistsReturns(errors.New("some-donor does not exist"))

args := []string{"some-donor", "-p", "some-plan"}
args := []string{"some-donor", "some-plan"}

err := plugin.Migrate(fakeMigrator, args)
Expect(err).To(MatchError("some-donor does not exist"))
Expand All @@ -120,11 +120,11 @@ var _ = Describe("Plugin Commands", func() {
It("returns an error if not enough args are passed", func() {
args := []string{"just-a-source"}
err := plugin.Migrate(fakeMigrator, args)
Expect(err).To(MatchError(migrateUsage + "\n\nthe required flag `-p, --plan' was not specified"))
Expect(err).To(MatchError(migrateUsage + "\n\nthe required argument `<p.mysql-plan-type>` was not provided"))
})

It("returns an error if too many args are passed", func() {
args := []string{"source", "-p", "plan-type", "extra-arg"}
args := []string{"source", "plan-type", "extra-arg"}
err := plugin.Migrate(fakeMigrator, args)
Expect(err).To(MatchError(migrateUsage + "\n\nunexpected arguments: extra-arg"))
})
Expand All @@ -141,15 +141,15 @@ var _ = Describe("Plugin Commands", func() {
})

It("returns an error and attempts to delete the new service instance", func() {
args := []string{"some-donor", "-p", "some-plan"}
args := []string{"some-donor", "some-plan"}
err := plugin.Migrate(fakeMigrator, args)
Expect(err).To(MatchError(MatchRegexp("error creating service instance: some-cf-error. Attempting to clean up service some-donor-new")))
Expect(fakeMigrator.CleanupOnErrorCallCount()).To(Equal(1))
})

It("returns an error and doesn't clean up when the --no-cleanup flag is passed", func() {
args := []string{
"some-donor", "-p", "some-plan", "--no-cleanup",
"some-donor", "some-plan", "--no-cleanup",
}

err := plugin.Migrate(fakeMigrator, args)
Expand All @@ -164,7 +164,7 @@ var _ = Describe("Plugin Commands", func() {
})

It("returns an error and attempts to delete the new service instance", func() {
args := []string{"some-donor", "-p", "some-plan"}
args := []string{"some-donor", "some-plan"}
err := plugin.Migrate(fakeMigrator, args)
Expect(err).To(MatchError(MatchRegexp("error migrating data: some-cf-error. Attempting to clean up service some-donor-new")))
opts := fakeMigrator.MigrateDataArgsForCall(0)
Expand All @@ -174,7 +174,7 @@ var _ = Describe("Plugin Commands", func() {

It("returns an error and doesn't clean up when the --no-cleanup flag is passed", func() {
args := []string{
"some-donor", "-p", "some-plan", "--no-cleanup",
"some-donor", "some-plan", "--no-cleanup",
}

err := plugin.Migrate(fakeMigrator, args)
Expand All @@ -193,7 +193,7 @@ var _ = Describe("Plugin Commands", func() {
})

It("returns an error and doesn't clean up regardless of --no-cleanup flag", func() {
args := []string{"some-donor", "-p", "some-plan"}
args := []string{"some-donor", "some-plan"}
err := plugin.Migrate(fakeMigrator, args)
Expect(err).To(MatchError("some-cf-error"))
Expect(fakeMigrator.MigrateDataCallCount()).To(Equal(1))
Expand Down
12 changes: 6 additions & 6 deletions specs/acceptance/migrate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ var _ = Describe("Migrate Integration Tests", func() {
})

It("fails on invalid donor service instance", func() {
cmd := exec.Command("cf", "mysql-tools", "migrate", "fake-donor-service", "-p", destPlan)
cmd := exec.Command("cf", "mysql-tools", "migrate", "fake-donor-service", destPlan)
session, err := gexec.Start(cmd, GinkgoWriter, GinkgoWriter)
Expect(err).NotTo(HaveOccurred())
Eventually(session, "1m", "1s").Should(gexec.Exit(1))
Expand Down Expand Up @@ -123,7 +123,7 @@ var _ = Describe("Migrate Integration Tests", func() {
serviceKey := test_helpers.GetServiceKey(sourceInstance, "database-key")
test_helpers.DeleteServiceKey(sourceInstance, "database-key")

cmd := exec.Command("cf", "mysql-tools", "migrate", sourceInstance, "-p", destPlan)
cmd := exec.Command("cf", "mysql-tools", "migrate", sourceInstance, destPlan)
session, err := gexec.Start(cmd, GinkgoWriter, GinkgoWriter)
Expect(err).NotTo(HaveOccurred())
Eventually(session, migrationTimeout, "1s").Should(gexec.Exit(0))
Expand Down Expand Up @@ -181,7 +181,7 @@ var _ = Describe("Migrate Integration Tests", func() {
})

It("doesn't delete the migration app when the --no-cleanup flag is specified", func() {
cmd := exec.Command("cf", "mysql-tools", "migrate", "--no-cleanup", sourceInstance, "-p", destPlan)
cmd := exec.Command("cf", "mysql-tools", "migrate", "--no-cleanup", sourceInstance, destPlan)
session, err := gexec.Start(cmd, GinkgoWriter, GinkgoWriter)
Expect(err).NotTo(HaveOccurred())
Eventually(session, migrationTimeout, "1s").Should(gexec.Exit(0))
Expand All @@ -193,7 +193,7 @@ var _ = Describe("Migrate Integration Tests", func() {
})

It("fails on invalid service plan", func() {
cmd := exec.Command("cf", "mysql-tools", "migrate", sourceInstance, "-p", "fake-service-plan")
cmd := exec.Command("cf", "mysql-tools", "migrate", sourceInstance, "fake-service-plan")
session, err := gexec.Start(cmd, GinkgoWriter, GinkgoWriter)
Expect(err).NotTo(HaveOccurred())
Eventually(session, "1m", "1s").Should(gexec.Exit(1))
Expand All @@ -207,7 +207,7 @@ var _ = Describe("Migrate Integration Tests", func() {
})

It("Deletes the recipient service instance", func() {
cmd := exec.Command("cf", "mysql-tools", "migrate", sourceInstance, "-p", destPlan)
cmd := exec.Command("cf", "mysql-tools", "migrate", sourceInstance, destPlan)
session, err := gexec.Start(cmd, GinkgoWriter, GinkgoWriter)
Expect(err).NotTo(HaveOccurred())

Expand All @@ -230,7 +230,7 @@ var _ = Describe("Migrate Integration Tests", func() {

It("Does not delete the recipient service instance when the --no-cleanup flag is specified", func() {
renamedSourceInstance = sourceInstance + "-old"
cmd := exec.Command("cf", "mysql-tools", "migrate", "--no-cleanup", sourceInstance, "-p", destPlan)
cmd := exec.Command("cf", "mysql-tools", "migrate", "--no-cleanup", sourceInstance, destPlan)
session, err := gexec.Start(cmd, GinkgoWriter, GinkgoWriter)
Expect(err).NotTo(HaveOccurred())

Expand Down
4 changes: 2 additions & 2 deletions specs/acceptance/multi_schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ var _ = Describe("Migrate Integration Tests v2", func() {
})

By("Migrating data using the migrate command", func() {
cmd := exec.Command("cf", "mysql-tools", "migrate", sourceInstance, "-p", destPlan)
cmd := exec.Command("cf", "mysql-tools", "migrate", sourceInstance, destPlan)
session, err := gexec.Start(cmd, GinkgoWriter, GinkgoWriter)
Expect(err).NotTo(HaveOccurred())
Eventually(session, migrationTimeout, "1s").Should(gexec.Exit(0))
Expand Down Expand Up @@ -126,7 +126,7 @@ var _ = Describe("Migrate Integration Tests v2", func() {
})

It("transfers all DBs", func() {
cmd := exec.Command("cf", "mysql-tools", "migrate", sourceInstance, "-p", destPlan)
cmd := exec.Command("cf", "mysql-tools", "migrate", sourceInstance, destPlan)
session, err := gexec.Start(cmd, GinkgoWriter, GinkgoWriter)
Expect(err).NotTo(HaveOccurred())
Eventually(session, migrationTimeout, "1s").Should(gexec.Exit(0))
Expand Down
2 changes: 1 addition & 1 deletion specs/acceptance/skip_tls_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ var _ = Describe("Skip TLS Validation", func() {
})

By("Migrating data using the migrate command with skip-tls-validation", func() {
cmd := exec.Command("cf", "mysql-tools", "migrate", sourceInstance, "-p", destPlan, "--skip-tls-validation")
cmd := exec.Command("cf", "mysql-tools", "migrate", sourceInstance, destPlan, "--skip-tls-validation")
session, err := gexec.Start(cmd, GinkgoWriter, GinkgoWriter)
Expect(err).NotTo(HaveOccurred())
Eventually(session, migrationTimeout, "1s").Should(gexec.Exit(0))
Expand Down
2 changes: 1 addition & 1 deletion specs/smoke_tests/smoke_tests_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ var _ = Describe("SmokeTests", func() {
})

By("Migrating data using the migrate command", func() {
cmd := exec.Command("cf", "mysql-tools", "migrate", instanceName, "-p", destPlan)
cmd := exec.Command("cf", "mysql-tools", "migrate", instanceName, destPlan)
session, err := gexec.Start(cmd, GinkgoWriter, GinkgoWriter)
Expect(err).NotTo(HaveOccurred())
Eventually(session, migrationTimeout, "1s").Should(gexec.Exit(0))
Expand Down

0 comments on commit e657c12

Please sign in to comment.