Skip to content

[cli] [vtctl] Migrate all vtctl commands to pflag #11320

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.

Sign up for GitHub

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

Merged
deepthi merged 16 commits into from
Oct 3, 2022
Merged

[cli] [vtctl] Migrate all vtctl commands to pflag #11320

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
8372e38
[cli] [vtctl] Migrate all vtctl commands to `pflag`
Sep 22, 2022
cb6a0ad
fix test flags
Sep 22, 2022
0866965
Handle subflag argument parsing without fully moving to cobra (yet!)
Sep 29, 2022
c859172
update examples
Sep 29, 2022
893482b
strip off double-dash
Sep 29, 2022
52582c5
Fix pflag getting tripped on shard 0 bounded shard names
Sep 29, 2022
7a99394
remove commented-out code
Sep 29, 2022
8868cea
update release notes
Sep 29, 2022
453b1b4
fix ExecuteHook, which takes no flags
Sep 30, 2022
3d44768
add deprecation notice around interspersal
Sep 30, 2022
dd4a4a4
fix flags for vtctld2 web calls
Sep 30, 2022
fb2ca46
Run make web_build
notfelineit Sep 30, 2022
87d23fd
test: fix vtctld_web_test
deepthi Sep 30, 2022
02cdc4c
update test assertions
Oct 1, 2022
1b4b4e0
just double-dash everything
Oct 1, 2022
80ba3fb
Run make web_build
notfelineit Oct 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions doc/releasenotes/15_0_0_summary.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
- The deprecated `--mutex-profile-fraction` flag has been removed. Please use `--pprof=mutex` instead.
- The deprecated vtgate/vtexplain/vtcombo flag `--planner_version` has been removed. Please use `--planner-version` instead.
- The deprecated flag `--master_connect_retry` has been removed. Please use `--replication_connect_retry` instead.
- `vtctl` commands that take shard names and ranges as positional arguments (e.g. `vtctl Reshard ks.workflow -80 -40,40-80`) need to have their positional arguments separated from their flag arguments by a double-dash separator to avoid the new parsing library from mistaking them as flags (e.g. `vtctl Reshard ks.workflow -- -80 -40,40-80`).
Copy link
Copy Markdown
Member

@mattlord mattlord Sep 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In total, I think we've made the double dash usage required to ensure correct behavior as part of this work (e.g. sub-command --help). IMO, we should make a more general announcement/summary that the double dash usage is required (even though in some cases things will work fine / the same w/o it).

Copy link
Copy Markdown
Contributor Author

@ajm188 ajm188 Sep 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm adding a deprecation notice to vtctl.RunCommand that i hope will help.


#### Vindex Interface

Expand Down Expand Up @@ -85,10 +86,10 @@ connections, similar to the way pooling happens for write buffers. Defaults to o

We introduced the ability to resume a VDiff2 workflow:
```
$ vtctlclient --server=localhost:15999 VDiff -- --v2 customer.commerce2customer resume 4c664dc2-eba9-11ec-9ef7-920702940ee0
$ vtctlclient --server=localhost:15999 VDiff --v2 customer.commerce2customer resume 4c664dc2-eba9-11ec-9ef7-920702940ee0
Copy link
Copy Markdown
Member

@mattlord mattlord Sep 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove the double dashes? Compare the output of these two things for an example of why I think we should always use them and why I feel the way I do about noting that it's (virtually) required now in the release notes:

$ vtctlclient --server=localhost:15999 VDiff --help
$ vtctlclient --server=localhost:15999 VDiff -- --help

This same comment applies to all of the other places we remove them in the PR.

Copy link
Copy Markdown
Contributor Author

@ajm188 ajm188 Sep 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because that gets in the way of moving to cobra (cobra does one full-blown parse by merging together the parent/subcommand flag-sets, so if you have the -- after the sub-command name then all the sub-command flags get treated as positional arguments instead of flags)

VDiff 4c664dc2-eba9-11ec-9ef7-920702940ee0 resumed on target shards, use show to view progress

$ vtctlclient --server=localhost:15999 VDiff -- --v2 customer.commerce2customer show last
$ vtctlclient --server=localhost:15999 VDiff --v2 customer.commerce2customer show last

VDiff Summary for customer.commerce2customer (4c664dc2-eba9-11ec-9ef7-920702940ee0)
State: completed
Expand All @@ -99,7 +100,7 @@ CompletedAt: 2022-06-26 22:44:31

Use "--format=json" for more detailed output.

$ vtctlclient --server=localhost:15999 VDiff -- --v2 --format=json customer.commerce2customer show last
$ vtctlclient --server=localhost:15999 VDiff --v2 --format=json customer.commerce2customer show last
{
"Workflow": "commerce2customer",
"Keyspace": "customer",
Expand Down
2 changes: 1 addition & 1 deletion examples/local/scripts/consul-up.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ sleep 5
echo "add $cell CellInfo"
set +e
# shellcheck disable=SC2086
vtctl $TOPOLOGY_FLAGS VtctldCommand AddCellInfo -- \
vtctl $TOPOLOGY_FLAGS VtctldCommand AddCellInfo \
--root "vitess/$cell" \
--server-address "${CONSUL_SERVER}:${consul_http_port}" \
"$cell"
Expand Down
2 changes: 1 addition & 1 deletion examples/local/scripts/etcd-up.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ etcdctl --endpoints "http://${ETCD_SERVER}" mkdir /vitess/$cell &
echo "add $cell CellInfo"
set +e
# shellcheck disable=SC2086
vtctl $TOPOLOGY_FLAGS VtctldCommand AddCellInfo -- \
vtctl $TOPOLOGY_FLAGS VtctldCommand AddCellInfo \
--root /vitess/$cell \
--server-address "${ETCD_SERVER}" \
$cell
Expand Down
2 changes: 1 addition & 1 deletion examples/local/scripts/k3s-up.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ $KUBECTL create -f ../../go/vt/topo/k8stopo/VitessTopoNodes-crd.yaml
# Add the CellInfo description for the cell
set +e
echo "add $cell CellInfo"
vtctl $TOPOLOGY_FLAGS VtctldCommand AddCellInfo -- \
vtctl $TOPOLOGY_FLAGS VtctldCommand AddCellInfo \
--root /vitess/$cell \
$cell
set -e
Expand Down
2 changes: 1 addition & 1 deletion examples/local/scripts/zk-up.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ echo "Started zk servers."
# If the node already exists, it's fine, means we used existing data.
set +e
# shellcheck disable=SC2086
vtctl $TOPOLOGY_FLAGS VtctldCommand AddCellInfo -- \
vtctl $TOPOLOGY_FLAGS VtctldCommand AddCellInfo \
--root /vitess/$cell \
--server-address $ZK_SERVER \
$cell
Expand Down
27 changes: 26 additions & 1 deletion go/cmd/vtctl/vtctl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,29 @@ var (

func init() {
servenv.OnParse(func(fs *pflag.FlagSet) {
// N.B. This is necessary for subcommand pflag parsing when not using
Copy link
Copy Markdown
Member

@mattlord mattlord Sep 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nota Bene? 🏅 😉

// cobra (cobra is where we're headed, but for `vtctl` it's a big lift
// before the RC cut).
//
// Essentially, the situation we have here is that commands look like:
//
// `vtctl [global flags] <command> [subcommand flags]`
//
// Since the default behavior of pflag is to allow "interspersed" flag
// and positional arguments, this means that the initial servenv parse
// will complain if _any_ subocmmand's flag is provided; for example, if
// you were to invoke
//
// `vtctl AddCellInfo --root /vitess/global --server_address "1.2.3.4" global
//
// then you would get the error "unknown flag --root", even though that
// is a valid flag for the AddCellInfo flag.
//
// By disabling interspersal on the top-level parse, anything after the
// command name ("AddCellInfo", in this example) will be forwarded to
// the subcommand's flag set for further parsing.
fs.SetInterspersed(false)
Comment on lines 73 to 76
Copy link
Copy Markdown
Member

@mattlord mattlord Sep 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I guess this is why you removed the double dashes? I still think we should leave them as the help output is still different, and it would be nice to have some uniformity across the client commands/binaries. Unless I'm misunderstanding something?

Copy link
Copy Markdown
Contributor Author

@ajm188 ajm188 Sep 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#11320 (comment)


logger := logutil.NewConsoleLogger()
fs.SetOutput(logutil.NewLoggerWriter(logger))
fs.Usage = func() {
Expand Down Expand Up @@ -156,12 +179,14 @@ func main() {
default:
log.Warningf("WARNING: vtctl should only be used for VDiff workflows. Consider using vtctldclient for all other commands.")

wr := wrangler.New(logutil.NewConsoleLogger(), ts, tmclient.NewTabletManagerClient())

if args[0] == "--" {
vtctl.PrintDoubleDashDeprecationNotice(wr)
args = args[1:]
}

action = args[0]
wr := wrangler.New(logutil.NewConsoleLogger(), ts, tmclient.NewTabletManagerClient())
err := vtctl.RunCommand(ctx, wr, args)
cancel()
switch err {
Expand Down
2 changes: 1 addition & 1 deletion go/test/endtoend/recovery/pitr/shardedpitr_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ func performResharding(t *testing.T) {
require.NoError(t, err)
}

err = clusterInstance.VtctlclientProcess.ExecuteCommand("Reshard", "--", "--v1", "ks.reshardWorkflow", "0", "-80,80-")
err = clusterInstance.VtctlclientProcess.ExecuteCommand("Reshard", "--", "--v1", "ks.reshardWorkflow", "0", "--", "-80,80-")
Copy link
Copy Markdown
Member

@mattlord mattlord Sep 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need both double dashes do we? Is the first one not enough?

Copy link
Copy Markdown
Contributor Author

@ajm188 ajm188 Sep 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#11320 (comment) sort of explains it. the double-dash breaks further parsing, so everything after it (--v1, in this case) because a positional argument. so you want this after your last "true" flag

require.NoError(t, err)

err = clusterInstance.VtctlclientProcess.ExecuteCommand("SwitchReads", "--", "--tablet_types=rdonly", "ks.reshardWorkflow")
Expand Down
2 changes: 1 addition & 1 deletion go/test/endtoend/recovery/pitrtls/shardedpitr_tls_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ func tlsPerformResharding(t *testing.T) {
require.NoError(t, err)
}

err = clusterInstance.VtctlclientProcess.ExecuteCommand("Reshard", "ks.reshardWorkflow", "0", "-80,80-")
err = clusterInstance.VtctlclientProcess.ExecuteCommand("Reshard", "ks.reshardWorkflow", "0", "--", "-80,80-")
Copy link
Copy Markdown
Collaborator

@deepthi deepthi Sep 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the only thing up for debate.
Looks like it affects both v1 and v2 versions of the VRepl workflows.
@mattlord what are your thoughts?
@ajm188 do we have any alternatives besides requiring people to provide the additional --?

Copy link
Copy Markdown
Collaborator

@deepthi deepthi Sep 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something we'll need to note in the release notes, so I've added that label.

Copy link
Copy Markdown
Member

@rohit-nayak-ps rohit-nayak-ps Sep 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't affect v2 workflows because the shards are passed as flags like vtctlclient Reshard -- --source_shards '0' --target_shards '-80,80-' Create customer.cust2cust where as in v1 workflows the shards are passed as Args like Reshard -- --v1 --cells=zone1 --tablet_types=replica,primary merchant-type.m2m3 -80,80- -40,40-c0,c0-

One possibility is to require quoting the shard arguments to the v1 workflows. That will require a trivial code change, however it will still make it backwardly incompatible. Since we have deprecated v1 workflows, the approach in this PR might be acceptable.

Copy link
Copy Markdown
Contributor Author

@ajm188 ajm188 Sep 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One possibility is to require quoting the shard arguments to the v1 workflows.

this doesn't work (i tried it)

$ go run ./go/cmd/pflagtester -80,80-
unknown shorthand flag: '8' in -80,80-
Usage of /var/folders/8q/6w7llb2j03v454_4pcm68w7w0000gn/T/go-build3458657591/b001/exe/pflagtester:
      --foo int    (default 2)
unknown shorthand flag: '8' in -80,80-
exit status 2
$ go run ./go/cmd/pflagtester "-80,80-"
unknown shorthand flag: '8' in -80,80-
Usage of /var/folders/8q/6w7llb2j03v454_4pcm68w7w0000gn/T/go-build4228639101/b001/exe/pflagtester:
      --foo int    (default 2)
unknown shorthand flag: '8' in -80,80-
exit status 2

Copy link
Copy Markdown
Contributor Author

@ajm188 ajm188 Sep 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rohit is correct thought that this only affects v1 workflows

Copy link
Copy Markdown
Member

@rohit-nayak-ps rohit-nayak-ps Sep 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One possibility is to require quoting the shard arguments to the v1 workflows. That will require a trivial code change, however it will still make it backwardly incompatible. Since we have deprecated v1 workflows, the approach in this PR might be acceptable.

I mean't after the code change referred to above, we can support quotes. But it just replaces one incompatibility with another ...

Copy link
Copy Markdown
Contributor Author

@ajm188 ajm188 Sep 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh, gotcha gotcha, sorry i misunderstood!

Copy link
Copy Markdown
Contributor Author

@ajm188 ajm188 Sep 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@deepthi a683a87 i also removed the double-dashes from the VDiff notes since those are no longer required (cc @mattlord)

require.NoError(t, err)

err = clusterInstance.VtctlclientProcess.ExecuteCommand("SwitchReads", "--", "--tablet_type=rdonly", "ks.reshardWorkflow")
Expand Down
2 changes: 1 addition & 1 deletion go/test/endtoend/vreplication/vreplication_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -841,7 +841,7 @@ func reshard(t *testing.T, ksName string, tableName string, workflow string, sou
t.Fatal(err)
}
}
if err := vc.VtctlClient.ExecuteCommand("Reshard", "--", "--v1", "--cells="+sourceCellOrAlias, "--tablet_types=replica,primary", ksWorkflow, sourceShards, targetShards); err != nil {
if err := vc.VtctlClient.ExecuteCommand("Reshard", "--", "--v1", "--cells="+sourceCellOrAlias, "--tablet_types=replica,primary", ksWorkflow, "--", sourceShards, targetShards); err != nil {
t.Fatalf("Reshard command failed with %+v\n", err)
}
tablets := vc.getVttabletsInKeyspace(t, defaultCell, ksName, "primary")
Expand Down
8 changes: 4 additions & 4 deletions go/test/endtoend/vtctldweb/vtctld_web_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,11 @@ func TestCreateKs(t *testing.T) {
case 0:
err := ele.SendKeys("test_keyspace3")
require.Nil(t, err)
assertDialogCommand(t, dialog, []string{"CreateKeyspace", "-force=false", "test_keyspace3"})
assertDialogCommand(t, dialog, []string{"CreateKeyspace", "--force=false", "test_keyspace3"})
}
}

assertDialogCommand(t, dialog, []string{"CreateKeyspace", "-force=false", "test_keyspace3"})
assertDialogCommand(t, dialog, []string{"CreateKeyspace", "--force=false", "test_keyspace3"})

create, err := dialog.FindElement(selenium.ByID, "vt-action")
require.Nil(t, err)
Expand Down Expand Up @@ -195,14 +195,14 @@ func TestDashboardValidate(t *testing.T) {
dialog, err := dashboardContent.FindElement(selenium.ByTagName, "vt-dialog")
require.Nil(t, err)

assertDialogCommand(t, dialog, []string{"Validate", "-ping-tablets=false"})
assertDialogCommand(t, dialog, []string{"Validate", "--ping-tablets=false"})

checkBoxes, err := dialog.FindElements(selenium.ByClassName, "md-checkbox-inner-container")
require.Nil(t, err)

click(t, checkBoxes[0])

assertDialogCommand(t, dialog, []string{"Validate", "-ping-tablets"})
assertDialogCommand(t, dialog, []string{"Validate", "--ping-tablets"})

validate, err := dialog.FindElement(selenium.ByID, "vt-action")
require.Nil(t, err)
Expand Down
2 changes: 1 addition & 1 deletion go/test/endtoend/vtgate/schema/schema_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ func testDropNonExistentTables(t *testing.T) {
func testCreateInvalidView(t *testing.T) {
for _, ddlStrategy := range []string{"direct", "direct -allow-zero-in-date"} {
createInvalidView := "CREATE OR REPLACE VIEW invalid_view AS SELECT * FROM nonexistent_table;"
output, err := clusterInstance.VtctlclientProcess.ExecuteCommandWithOutput("ApplySchema", "--", "-skip_preflight", "-ddl_strategy", ddlStrategy, "--sql", createInvalidView, keyspaceName)
output, err := clusterInstance.VtctlclientProcess.ExecuteCommandWithOutput("ApplySchema", "--", "--skip_preflight", "--ddl_strategy", ddlStrategy, "--sql", createInvalidView, keyspaceName)
require.Error(t, err)
assert.Contains(t, output, "doesn't exist (errno 1146)")
}
Expand Down
12 changes: 6 additions & 6 deletions go/vt/vtctl/backup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ package vtctl

import (
"context"
"flag"
"fmt"
"time"

"github.com/spf13/pflag"
"google.golang.org/grpc"

"vitess.io/vitess/go/protoutil"
Expand Down Expand Up @@ -70,7 +70,7 @@ func init() {
})
}

func commandBackup(ctx context.Context, wr *wrangler.Wrangler, subFlags *flag.FlagSet, args []string) error {
func commandBackup(ctx context.Context, wr *wrangler.Wrangler, subFlags *pflag.FlagSet, args []string) error {
concurrency := subFlags.Int("concurrency", 4, "Specifies the number of compression/checksum jobs to run simultaneously")
allowPrimary := subFlags.Bool("allow_primary", false, "Allows backups to be taken on primary. Warning!! If you are using the builtin backup engine, this will shutdown your primary mysql for as long as it takes to create a backup.")

Expand Down Expand Up @@ -108,7 +108,7 @@ func (b *backupEventStreamLogger) Send(resp *vtctldatapb.BackupResponse) error {
return nil
}

func commandBackupShard(ctx context.Context, wr *wrangler.Wrangler, subFlags *flag.FlagSet, args []string) error {
func commandBackupShard(ctx context.Context, wr *wrangler.Wrangler, subFlags *pflag.FlagSet, args []string) error {
concurrency := subFlags.Int("concurrency", 4, "Specifies the number of compression/checksum jobs to run simultaneously")
allowPrimary := subFlags.Bool("allow_primary", false, "Whether to use primary tablet for backup. Warning!! If you are using the builtin backup engine, this will shutdown your primary mysql for as long as it takes to create a backup.")

Expand All @@ -132,7 +132,7 @@ func commandBackupShard(ctx context.Context, wr *wrangler.Wrangler, subFlags *fl
}, &backupEventStreamLogger{logger: wr.Logger(), ctx: ctx})
}

func commandListBackups(ctx context.Context, wr *wrangler.Wrangler, subFlags *flag.FlagSet, args []string) error {
func commandListBackups(ctx context.Context, wr *wrangler.Wrangler, subFlags *pflag.FlagSet, args []string) error {
if err := subFlags.Parse(args); err != nil {
return err
}
Expand Down Expand Up @@ -161,7 +161,7 @@ func commandListBackups(ctx context.Context, wr *wrangler.Wrangler, subFlags *fl
return nil
}

func commandRemoveBackup(ctx context.Context, wr *wrangler.Wrangler, subFlags *flag.FlagSet, args []string) error {
func commandRemoveBackup(ctx context.Context, wr *wrangler.Wrangler, subFlags *pflag.FlagSet, args []string) error {
if err := subFlags.Parse(args); err != nil {
return err
}
Expand Down Expand Up @@ -198,7 +198,7 @@ func (b *backupRestoreEventStreamLogger) Send(resp *vtctldatapb.RestoreFromBacku
return nil
}

func commandRestoreFromBackup(ctx context.Context, wr *wrangler.Wrangler, subFlags *flag.FlagSet, args []string) error {
func commandRestoreFromBackup(ctx context.Context, wr *wrangler.Wrangler, subFlags *pflag.FlagSet, args []string) error {
backupTimestampStr := subFlags.String("backup_timestamp", "", "Use the backup taken at or before this timestamp rather than using the latest backup.")
if err := subFlags.Parse(args); err != nil {
return err
Expand Down
13 changes: 7 additions & 6 deletions go/vt/vtctl/cell_info.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@ package vtctl

import (
"context"
"flag"
"fmt"
"strings"

"github.com/spf13/pflag"

"vitess.io/vitess/go/vt/wrangler"

topodatapb "vitess.io/vitess/go/vt/proto/topodata"
Expand Down Expand Up @@ -71,7 +72,7 @@ func init() {
})
}

func commandAddCellInfo(ctx context.Context, wr *wrangler.Wrangler, subFlags *flag.FlagSet, args []string) error {
func commandAddCellInfo(ctx context.Context, wr *wrangler.Wrangler, subFlags *pflag.FlagSet, args []string) error {
serverAddress := subFlags.String("server_address", "", "The address the topology server is using for that cell.")
root := subFlags.String("root", "", "The root path the topology server is using for that cell.")
if err := subFlags.Parse(args); err != nil {
Expand All @@ -92,7 +93,7 @@ func commandAddCellInfo(ctx context.Context, wr *wrangler.Wrangler, subFlags *fl
return err
}

func commandUpdateCellInfo(ctx context.Context, wr *wrangler.Wrangler, subFlags *flag.FlagSet, args []string) error {
func commandUpdateCellInfo(ctx context.Context, wr *wrangler.Wrangler, subFlags *pflag.FlagSet, args []string) error {
serverAddress := subFlags.String("server_address", "", "The address the topology server is using for that cell.")
root := subFlags.String("root", "", "The root path the topology server is using for that cell.")
if err := subFlags.Parse(args); err != nil {
Expand All @@ -113,7 +114,7 @@ func commandUpdateCellInfo(ctx context.Context, wr *wrangler.Wrangler, subFlags
return err
}

func commandDeleteCellInfo(ctx context.Context, wr *wrangler.Wrangler, subFlags *flag.FlagSet, args []string) error {
func commandDeleteCellInfo(ctx context.Context, wr *wrangler.Wrangler, subFlags *pflag.FlagSet, args []string) error {
force := subFlags.Bool("force", false, "Proceeds even if the cell's topology server cannot be reached. The assumption is that you turned down the entire cell, and just need to update the global topo data.")
if err := subFlags.Parse(args); err != nil {
return err
Expand All @@ -130,7 +131,7 @@ func commandDeleteCellInfo(ctx context.Context, wr *wrangler.Wrangler, subFlags
return err
}

func commandGetCellInfoNames(ctx context.Context, wr *wrangler.Wrangler, subFlags *flag.FlagSet, args []string) error {
func commandGetCellInfoNames(ctx context.Context, wr *wrangler.Wrangler, subFlags *pflag.FlagSet, args []string) error {
if err := subFlags.Parse(args); err != nil {
return err
}
Expand All @@ -145,7 +146,7 @@ func commandGetCellInfoNames(ctx context.Context, wr *wrangler.Wrangler, subFlag
return nil
}

func commandGetCellInfo(ctx context.Context, wr *wrangler.Wrangler, subFlags *flag.FlagSet, args []string) error {
func commandGetCellInfo(ctx context.Context, wr *wrangler.Wrangler, subFlags *pflag.FlagSet, args []string) error {
if err := subFlags.Parse(args); err != nil {
return err
}
Expand Down
29 changes: 14 additions & 15 deletions go/vt/vtctl/cells_aliases.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@ package vtctl

import (
"context"
"flag"
"fmt"
"strings"

"github.com/spf13/pflag"

"vitess.io/vitess/go/vt/wrangler"

topodatapb "vitess.io/vitess/go/vt/proto/topodata"
Expand Down Expand Up @@ -64,53 +65,51 @@ func init() {
})
}

func commandAddCellsAlias(ctx context.Context, wr *wrangler.Wrangler, subFlags *flag.FlagSet, args []string) error {
cellsString := subFlags.String("cells", "", "The list of cell names that are members of this alias.")
func commandAddCellsAlias(ctx context.Context, wr *wrangler.Wrangler, subFlags *pflag.FlagSet, args []string) error {
cells := subFlags.StringSlice("cells", nil, "The list of cell names that are members of this alias.")
if err := subFlags.Parse(args); err != nil {
return err
}
if subFlags.NArg() != 1 {
return fmt.Errorf("the <alias> argument is required for the AddCellsAlias command")
}

cells := strings.Split(*cellsString, ",")
for i, cell := range cells {
cells[i] = strings.TrimSpace(cell)
for i, cell := range *cells {
(*cells)[i] = strings.TrimSpace(cell)
}

alias := subFlags.Arg(0)
_, err := wr.VtctldServer().AddCellsAlias(ctx, &vtctldatapb.AddCellsAliasRequest{
Name: alias,
Cells: cells,
Cells: *cells,
})
return err
}

func commandUpdateCellsAlias(ctx context.Context, wr *wrangler.Wrangler, subFlags *flag.FlagSet, args []string) error {
cellsString := subFlags.String("cells", "", "The list of cell names that are members of this alias.")
func commandUpdateCellsAlias(ctx context.Context, wr *wrangler.Wrangler, subFlags *pflag.FlagSet, args []string) error {
cells := subFlags.StringSlice("cells", nil, "The list of cell names that are members of this alias.")
if err := subFlags.Parse(args); err != nil {
return err
}
if subFlags.NArg() != 1 {
return fmt.Errorf("the <alias> argument is required for the UpdateCellsAlias command")
}

cells := strings.Split(*cellsString, ",")
for i, cell := range cells {
cells[i] = strings.TrimSpace(cell)
for i, cell := range *cells {
(*cells)[i] = strings.TrimSpace(cell)
}

alias := subFlags.Arg(0)
_, err := wr.VtctldServer().UpdateCellsAlias(ctx, &vtctldatapb.UpdateCellsAliasRequest{
Name: alias,
CellsAlias: &topodatapb.CellsAlias{
Cells: cells,
Cells: *cells,
},
})
return err
}

func commandDeleteCellsAlias(ctx context.Context, wr *wrangler.Wrangler, subFlags *flag.FlagSet, args []string) error {
func commandDeleteCellsAlias(ctx context.Context, wr *wrangler.Wrangler, subFlags *pflag.FlagSet, args []string) error {
if err := subFlags.Parse(args); err != nil {
return err
}
Expand All @@ -125,7 +124,7 @@ func commandDeleteCellsAlias(ctx context.Context, wr *wrangler.Wrangler, subFlag
return err
}

func commandGetCellsAliases(ctx context.Context, wr *wrangler.Wrangler, subFlags *flag.FlagSet, args []string) error {
func commandGetCellsAliases(ctx context.Context, wr *wrangler.Wrangler, subFlags *pflag.FlagSet, args []string) error {
if err := subFlags.Parse(args); err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions go/vt/vtctl/endtoend/get_schema_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ func TestGetSchema(t *testing.T) {
utils.MustMatch(t, sd, actual)

// reset for the next invocation, where we verify that passing
// -table_sizes_only does not include the create table statement or columns.
// --table_sizes_only does not include the create table statement or columns.
logger.Events = nil
sd = &tabletmanagerdatapb.SchemaDefinition{
TableDefinitions: []*tabletmanagerdatapb.TableDefinition{
Expand All @@ -202,7 +202,7 @@ func TestGetSchema(t *testing.T) {

err = vtctl.RunCommand(ctx, wrangler.New(logger, topo, &tmc), []string{
"GetSchema",
"-table_sizes_only",
"--table_sizes_only",
topoproto.TabletAliasString(tablet.Alias),
})
require.NoError(t, err)
Expand Down
Loading