Skip to content
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

Add info subcommand to CLI #17566

Merged
merged 7 commits into from
Jun 7, 2023
Merged

Conversation

Xenorith
Copy link
Contributor

@Xenorith Xenorith commented Jun 6, 2023

Adds info subcommand, which contains:

  • cache: worker capacity information, calls the existing bin/alluxio fsadmin report capacity command
  • collect: collects cluster information into a single tarball, calls the existing bin/alluxio collectInfo command
  • master: master quorum information, calls the existing bin/alluxio fs masterInfo command
  • report: cluster information, calls the existing bin/alluxio fsadmin report command, excluding the capacity category

#17522

@Xenorith Xenorith requested a review from jenoudet June 6, 2023 21:40
@Xenorith
Copy link
Contributor Author

Xenorith commented Jun 6, 2023

note this is far from the desired state for these commands, but additional work would require changing the underlying java implementation

@Xenorith Xenorith requested a review from jingwenouyang June 6, 2023 21:45
Copy link
Contributor

@jenoudet jenoudet left a comment

Choose a reason for hiding this comment

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

Also, I think it's worth changing the global --debugLog flag to either --debug-log or just --debug.

})
cmd.Flags().BoolVar(&c.liveWorkers, "live", false, "Only show live workers for capacity report")
cmd.Flags().BoolVar(&c.lostWorkers, "lost", false, "Only show lost workers for capacity report")
cmd.Flags().StringSliceVar(&c.workersList, "workers", nil, "Only show specified workers for capacity report, labeled by hostname or IP address")
Copy link
Contributor

Choose a reason for hiding this comment

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

StringSliceVar supports using --workers multiple times to specify multiple workers, maybe it should be named --worker (singular) then so the command can read bin/cli.sh info cache --worker <hostname1> --worker <hostname2> ... (similar to bin/cli.sh conf log --target <target1> --target <target2>...). wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

worker it is


func (c *CollectCommand) ToCommand() *cobra.Command {
cmd := c.Base().InitRunJavaClassCmd(&cobra.Command{
Use: fmt.Sprintf("%v [command] [outputPath]", Collect.CommandName),
Copy link
Contributor

Choose a reason for hiding this comment

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

The current command currently looks like bin/cli.sh info collect {all | collectAlluxioInfo. | collectConfig | collectEnv | collectJvmInfo | collectLog | collectMetrics} /path/to/output. Here are some suggestions for improvements:
Trim commands to become bin/cli.sh info collect {all | cluster | conf | env | jvm | log | metrics }. Turn the output path into a flag like -o, --output-path, make the default value pwd or make flag mandatory.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agree that avoid using arguments in general. adding a required --output-path

Copy link
Contributor Author

@Xenorith Xenorith left a comment

Choose a reason for hiding this comment

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

i saw an existing --debug flag for adding debug java opts. so i want to avoid using --debug altogether; --debug-logging for showing the CLI debug logging and then --debug-opts for the java opts side.

})
cmd.Flags().BoolVar(&c.liveWorkers, "live", false, "Only show live workers for capacity report")
cmd.Flags().BoolVar(&c.lostWorkers, "lost", false, "Only show lost workers for capacity report")
cmd.Flags().StringSliceVar(&c.workersList, "workers", nil, "Only show specified workers for capacity report, labeled by hostname or IP address")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

worker it is


func (c *CollectCommand) ToCommand() *cobra.Command {
cmd := c.Base().InitRunJavaClassCmd(&cobra.Command{
Use: fmt.Sprintf("%v [command] [outputPath]", Collect.CommandName),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

agree that avoid using arguments in general. adding a required --output-path

@jenoudet
Copy link
Contributor

jenoudet commented Jun 7, 2023

I'm getting errors when running basic commands

$ bin/cli.sh info
panic: Failed to find flag "workers" and mark it as being in a mutually exclusive flag group

goroutine 1 [running]:
github.com/spf13/cobra.(*Command).MarkFlagsMutuallyExclusive(0x103180d80?, {0x14000141d48, 0x3, 0x3})
	/Users/arthurjenoudet/git/pkg/mod/github.com/spf13/cobra@v1.7.0/flag_groups.go:53 +0x224
alluxio.org/cli/cmd/info.(*CacheCommand).ToCommand(0x10317c8c0)
	/Users/arthurjenoudet/git/alluxio/cli/src/alluxio.org/cli/cmd/info/cache.go:54 +0x1f4
alluxio.org/cli/env.(*Service).InitCommandTree(0x10317cb00, 0x102e786e3?)
	/Users/arthurjenoudet/git/alluxio/cli/src/alluxio.org/cli/env/service.go:50 +0xfc
alluxio.org/cli/env.InitServiceCommandTree(...)
	/Users/arthurjenoudet/git/alluxio/cli/src/alluxio.org/cli/env/service.go:32
alluxio.org/cli/launch.Run()
	/Users/arthurjenoudet/git/alluxio/cli/src/alluxio.org/cli/launch/launch.go:48 +0x22c
main.main()
	/Users/arthurjenoudet/git/alluxio/cli/src/alluxio.org/cli/main.go:46 +0x170

Are you getting this as well?

@Xenorith
Copy link
Contributor Author

Xenorith commented Jun 7, 2023

I'm getting errors when running basic commands

$ bin/cli.sh info
panic: Failed to find flag "workers" and mark it as being in a mutually exclusive flag group

goroutine 1 [running]:
github.com/spf13/cobra.(*Command).MarkFlagsMutuallyExclusive(0x103180d80?, {0x14000141d48, 0x3, 0x3})
	/Users/arthurjenoudet/git/pkg/mod/github.com/spf13/cobra@v1.7.0/flag_groups.go:53 +0x224
alluxio.org/cli/cmd/info.(*CacheCommand).ToCommand(0x10317c8c0)
	/Users/arthurjenoudet/git/alluxio/cli/src/alluxio.org/cli/cmd/info/cache.go:54 +0x1f4
alluxio.org/cli/env.(*Service).InitCommandTree(0x10317cb00, 0x102e786e3?)
	/Users/arthurjenoudet/git/alluxio/cli/src/alluxio.org/cli/env/service.go:50 +0xfc
alluxio.org/cli/env.InitServiceCommandTree(...)
	/Users/arthurjenoudet/git/alluxio/cli/src/alluxio.org/cli/env/service.go:32
alluxio.org/cli/launch.Run()
	/Users/arthurjenoudet/git/alluxio/cli/src/alluxio.org/cli/launch/launch.go:48 +0x22c
main.main()
	/Users/arthurjenoudet/git/alluxio/cli/src/alluxio.org/cli/main.go:46 +0x170

Are you getting this as well?

doh i know what's wrong

@Xenorith Xenorith added type-feature This issue is a feature request area-shell Alluxio Command Line Interface labels Jun 7, 2023
@Xenorith
Copy link
Contributor Author

Xenorith commented Jun 7, 2023

alluxio-bot, merge this please

@alluxio-bot alluxio-bot merged commit 2e971b6 into Alluxio:golangCli Jun 7, 2023
Xenorith added a commit that referenced this pull request Aug 3, 2023
Adds `info` subcommand, which contains:
- `cache`: worker capacity information, calls the existing `bin/alluxio fsadmin report capacity` command
- `collect`: collects cluster information into a single tarball, calls the existing `bin/alluxio collectInfo` command
- `master`: master quorum information, calls the existing `bin/alluxio fs masterInfo` command
- `report`: cluster information, calls the existing `bin/alluxio fsadmin report` command, excluding the `capacity` category

#17522
			pr-link: #17566
			change-id: cid-2483fdb693f1572b0bf9c798503ea9cfd8e52b1d
Xenorith added a commit that referenced this pull request Aug 11, 2023
Adds `info` subcommand, which contains:
- `cache`: worker capacity information, calls the existing `bin/alluxio fsadmin report capacity` command
- `collect`: collects cluster information into a single tarball, calls the existing `bin/alluxio collectInfo` command
- `master`: master quorum information, calls the existing `bin/alluxio fs masterInfo` command
- `report`: cluster information, calls the existing `bin/alluxio fsadmin report` command, excluding the `capacity` category

#17522
			pr-link: #17566
			change-id: cid-2483fdb693f1572b0bf9c798503ea9cfd8e52b1d
twalluxio pushed a commit to twalluxio/alluxio that referenced this pull request Aug 14, 2023
Adds `info` subcommand, which contains:
- `cache`: worker capacity information, calls the existing `bin/alluxio fsadmin report capacity` command
- `collect`: collects cluster information into a single tarball, calls the existing `bin/alluxio collectInfo` command
- `master`: master quorum information, calls the existing `bin/alluxio fs masterInfo` command
- `report`: cluster information, calls the existing `bin/alluxio fsadmin report` command, excluding the `capacity` category

Alluxio#17522
			pr-link: Alluxio#17566
			change-id: cid-2483fdb693f1572b0bf9c798503ea9cfd8e52b1d
Xenorith added a commit that referenced this pull request Aug 15, 2023
- Add cli/ folder containing golang code
- Add script to compile code to executable in build/cli/build-cli.sh
- Add entrypoint script for development in bin/cli.sh
- Add build profile to build the CLI as part of tha maven build via `-PgoCli`
- Add `conf` service, consisting of
  - `bin/cli.sh conf get` as an example command, equivalent to `bin/alluxio getConf`
  - `bin/cli.sh conf log --name fully.qualified.class.path`, equivalent to `bin/alluxio fsadmin logLevel --logName fully.qualified.class.path`

See #17522 for more background info

			pr-link: #17532

dev docs for defining conventions for the new alluxio cli in golang

			pr-link: #17530

- Add commands to start/stop individual processes, ex. `bin/cli.sh process start master`
- Define process interface and registry
  - process specific java opts are defined with the process and are dynamically added to env
- Add journal format command as part of starting master process
- Mount options for worker are not ported because they will be deprecated in the near future

			pr-link: #17561

Adds `info` subcommand, which contains:
- `cache`: worker capacity information, calls the existing `bin/alluxio fsadmin report capacity` command
- `collect`: collects cluster information into a single tarball, calls the existing `bin/alluxio collectInfo` command
- `master`: master quorum information, calls the existing `bin/alluxio fs masterInfo` command
- `report`: cluster information, calls the existing `bin/alluxio fsadmin report` command, excluding the `capacity` category

			pr-link: #17566

Add journal commands to CLI as part of #17522
			pr-link: #17569

Add quorum/HA related commands as part of #17522
			pr-link: #17570

Add fs commands to golang CLI as part of #17522

unlike other CLI commands, utilize arguments to maintain the existing structure of filesystem commands (ex. cp, du, ls, mv, rm, etc)

			pr-link: #17580

Add generate commands to golang CLI as part of #17522

bin/alluxio docGen -> bin/cli generate docs
bin/alluxio bootstrapConf -> bin/cli generate template

to read ALLUXIO_CONF_DIR, also conducted a minor change on env variables.

			pr-link: #17790

Add exec commands to golang CLI as part of #17522

`bin/alluxio runClass` -> `bin/cli exec class`
`bin/alluxio runTests` -> `bin/cli exec testRun`
`bin/alluxio runUfsTests` -> `bin/cli exec testUfs`
`bin/alluxio runUfsIOTest` -> `bin/cli exec testUfsIO`
`bin/alluxio runHdfsMountTests` -> `bin/cli exec testHdfsMount`
`bin/alluxio runHmsTests` -> `bin/cli exec testHms`
`bin/alluxio runJournalCrashTest` -> `bin/cli exec testJournalCrash`

For command `exec test`, using different `--name` flags can lead to huge difference in other flags and options.
To manage flags and options, currently using different commands for different test types.
			pr-link: #17797

Prepare golang cli for tarball build and replace bin/alluxio

allow java 8 or 11

add cli binaries to other tarball profiles

Add `process` commands with multiple nodes to golang CLI as part of #17522

`bin/alluxio-start.sh masters` -> `bin/cli.sh process start masters`
`bin/alluxio-start.sh job_masters` -> `bin/cli.sh process start job_masters`
`bin/alluxio-start.sh workers` -> `bin/cli.sh process start workers`
`bin/alluxio-start.sh job_workers` -> `bin/cli.sh process start job_workers`
`bin/alluxio-start.sh proxies` -> `bin/cli.sh process start proxies`
`bin/alluxio-start.sh all` -> `bin/cli.sh process start all`
`bin/alluxio-start.sh local` -> `bin/cli.sh process start local`

`bin/alluxio-stop.sh masters` -> `bin/cli.sh process stop masters`
`bin/alluxio-stop.sh job_masters` -> `bin/cli.sh process stop job_masters`
`bin/alluxio-stop.sh workers` -> `bin/cli.sh process stop workers`
`bin/alluxio-stop.sh job_workers` -> `bin/cli.sh process stop job_workers`
`bin/alluxio-stop.sh proxies` -> `bin/cli.sh process stop proxies`
`bin/alluxio-stop.sh all` -> `bin/cli.sh process stop all`
`bin/alluxio-stop.sh local` -> `bin/cli.sh process stop local`

For command `process start/stop` on multiple nodes, using crypto's `ssh` package to create an SSH session, connect to masters, workers or all nodes, then send according subcommand on single nodes to these nodes.
			pr-link: #17887

Add job commands to golang CLI as part of #17522

`bin/alluxio-bash job cancel id` -> `bin/alluxio job cancel --id`
`bin/alluxio-bash job leader` -> `bin/alluxio job leader`
`bin/alluxio-bash job ls` -> `bin/alluxio job list`
`bin/alluxio-bash job getCmdStatus jobControlId` -> `bin/alluxio job cmdStatus --id`
`bin/alluxio-bash job stat [-v] id` -> `bin/alluxio job jobStatus [-v] --id`
`bin/alluxio-bash fs distributedCp src dst` -> `bin/alluxio job submit --type cp --src --dst`
`bin/alluxio-bash fs distributedMv src dst` -> `bin/alluxio job submit --type mv --src --dst`
`bin/alluxio-bash fs load path --submit` -> `bin/alluxio job load --path`

			pr-link: #17931

Clean up multiprocess code

- refactor commands launching the same process on multiple hosts into a single struct
- simplify the logic to ssh into different hosts with more straightforward handling of error channel
- move names.go under its own package to avoid import cycles
			pr-link: #17947

Add an `info version` subcommand to golang CLI as part of #17522

`bin/alluxio-bash version` -> `bin/alluxio info version`
			pr-link: #17943

update calls to old bash scripts to refer to alluxio-bash

Add `cache` commands to golang CLI as part of #17522
`bin/alluxio-bash formatWorker` -> `bin/cli cache format`
`bin/alluxio-bash fs freeWorker` -> `bin/cli cache free --worker`
`bin/alluxio-bash fs free` -> `bin/cli cache free [-f] --path`
`bin/alluxio-bash fs persist` -> `bin/cli cache persist --path`
`bin/alluxio-bash fs setTtl` -> `bin/cli cache ttl --set --duration --action`
`bin/alluxio-bash fs unsetTtl` -> `bin/cli cache ttl --unset`
			pr-link: #17937

backcompat support for start/stop scripts

skip mount args in start script backcompat

remove unsupported cmds
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-shell Alluxio Command Line Interface type-feature This issue is a feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants