Add CLI as a golang project #17532

Xenorith · 2023-06-01T05:33:45Z

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

- 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 the `bin/cli.sh conf get` as an example command, equivalent to `bin/alluxio getConf` See Alluxio#17522 for more background info

Xenorith · 2023-06-01T07:03:34Z

note this is merging into a feature branch, not to main

jenoudet · 2023-06-01T17:30:27Z

cli/src/alluxio.org/cli/cmd/conf/get.go

+	cmd.Flags().BoolVar(&c.DebugMode, "attachDebug", false, "True to attach debug opts")
+	cmd.Flags().StringVar(&c.InlineJavaOpts, "javaOpts", "", `Java options to apply, ex. "-Dkey=value"`)


After the camelCase vs dash-separated discussion we had, maybe we can use:

Suggested change

cmd.Flags().BoolVar(&c.DebugMode, "attachDebug", false, "True to attach debug opts")

cmd.Flags().StringVar(&c.InlineJavaOpts, "javaOpts", "", `Java options to apply, ex. "-Dkey=value"`)

cmd.Flags().BoolVarP(&c.DebugMode, "attach-debug", "d", false, "True to attach debug opts")

cmd.Flags().StringVarP(&c.InlineJavaOpts, "java-opts", "o", "", `Java options to apply, ex. "-Dkey=value"`)

This also adds one-letter shorthands for each options.

i used -D for java opts to exactly match what it was previously and -d for attach debug. now no one can use ds

right right, d is a bit overloaded correct. the general idea is that, as we get more familiar with the commands it'll be nice to have one-letter shorthands to use so that we can type less.

jenoudet · 2023-06-01T17:33:11Z

cli/src/alluxio.org/cli/cmd/conf/get.go

+
+func (c *GetCommand) ToCommand() *cobra.Command {
+	cmd := &cobra.Command{
+		Use:   fmt.Sprintf("%v [key]", Get.CommandName),


I'm surprise we have to write out the the "help" format ourselves. In python for example, when creation an argument, one can specify if it is required or optional, repeated, etc... when creating the option. The python library then take care of generation the help menu.
Is there a way to do that here? I worry this will lead to a great fragmentation and inconsistency of help menus, which is one of the main goals of rewriting the cli in Go (i.e. consistency and repeatability).

help info for flags are autogenerated, but not for args. this is what the help menu looks like without specifying the [key] portion of the Use string

$ bin/cli.sh conf get -h Look up a configuration value by its key or print all configuration if no key is provided Usage: conf get [flags] Flags: --attach-debug True to attach debug opts -h, --help help for get -D, --java-opts strings Alluxio properties to apply, ex. -Dkey=value Global Flags: --debugLog True to enable debug logging

so it makes sense that we have to describe the arguments. in this example, i've specified java-opts as a repeatable flag which cobra indicates as strings rather than string

help info for flags are autogenerated, but not for args. this is what the help menu looks like without specifying the [key] portion of the Use string

$ bin/cli.sh conf get -h Look up a configuration value by its key or print all configuration if no key is provided Usage: conf get [flags] Flags: --attach-debug True to attach debug opts -h, --help help for get -D, --java-opts strings Alluxio properties to apply, ex. -Dkey=value Global Flags: --debugLog True to enable debug logging

so it makes sense that we have to describe the arguments. in this example, i've specified java-opts as a repeatable flag which cobra indicates as strings rather than string for a simple string flag

jenoudet · 2023-06-01T17:37:51Z

cli/src/alluxio.org/cli/env/command.go

+
+type BaseCommand struct {
+	CommandName   string
+	JavaClassName string


I like this: it keeps the go cli command right next to the Java class name, it's clever.

jenoudet · 2023-06-01T17:38:44Z

cli/src/alluxio.org/cli/cmd/conf/log.go

+
+func (c *LogCommand) ToCommand() *cobra.Command {
+	cmd := &cobra.Command{
+		Use:   fmt.Sprintf("%v --name <name> [--level <level>] [--target <target>]", Log.CommandName),


ditto to my previous comment, I wish the Use argument could be auto-generated based on the flags the command registers.

jenoudet · 2023-06-01T17:42:01Z

cli/src/alluxio.org/cli/env/service.go

+
+var serviceRegistry = map[string]*Service{}
+
+func RegisterService(p *Service) *Service {


Service may be ambiguous with the Overmind naming. These docs use the word "object", but we could come up with something else if we wanted.

what's overmind? :)

jenoudet · 2023-06-01T17:46:24Z

cli/src/alluxio.org/cli/cmd/conf/log.go

+		panic(err)
+	}
+	cmd.Flags().StringVar(&c.Level, "level", "", "If specified, sets the specified logger at the given level")
+	cmd.Flags().StringVar(&c.Target, "target", "", "A list of comma delimited targets among <master|workers|job_master|job_workers|host:webPort[:role]>. Defaults to master,workers,job_master,job_workers")


I think a more user-friendly way to implement this argument would be to allow the --target flag to be repeated. E.g. bin/cli conf log ... --target master --target workers --target localhost:19999:master. This would allow for easier entry for the user, lesser risk they might omit a comma accidentally, or leave a space behind a comma, etc... In addition, it would allow greater flexibility when processing and the arguments from the Go code side.

This would also allow input validation by restricting the possible inputs to a known set of values (i.e. master|workers|job_master|job_workers|host:webPort[:role])

spf13/cobra#661 (comment), using StringSlice* lets use support for repeated flag, as well as comma-separated simultaneously.

this also starts the debate of who's job is it to verify the user inputs, go side or java side? we shouldn't do both...

it's really input validation, but more different, more user-friendly ways of inputting the information

Xenorith · 2023-06-02T19:46:15Z

alluxio-bot, merge this please

alluxio-bot · 2023-06-02T19:46:16Z

merge failed:
Merge refused because pull request does not have label start with type-

Xenorith · 2023-06-02T19:47:33Z

alluxio-bot, merge this please

- 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 change-id: cid-9e3bf70e1a20848e2e4ec59b08b3de00bebc527d

- 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 Alluxio#17522 for more background info pr-link: Alluxio#17532 change-id: cid-9e3bf70e1a20848e2e4ec59b08b3de00bebc527d

- 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

Xenorith requested a review from jenoudet June 1, 2023 05:33

add license header

cfa11e7

renames and add log command to complete conf service

9119d8e

Xenorith requested a review from jingwenouyang June 1, 2023 09:24

jenoudet reviewed Jun 1, 2023

View reviewed changes

jenoudet suggested changes Jun 1, 2023

View reviewed changes

Xenorith added 2 commits June 1, 2023 11:22

address comments

7496db2

resolve TODO for remaining flags in getConf

9d94fed

jenoudet approved these changes Jun 2, 2023

View reviewed changes

Xenorith added the type-ease-of-use This issue is about of how to improve the ease of use of Alluxio product label Jun 2, 2023

alluxio-bot merged commit 1118ce4 into Alluxio:golangCli Jun 2, 2023

Xenorith added area-shell Alluxio Command Line Interface type-feature This issue is a feature request and removed type-ease-of-use This issue is about of how to improve the ease of use of Alluxio product labels Jun 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add CLI as a golang project #17532

Add CLI as a golang project #17532

Xenorith commented Jun 1, 2023 •

edited

Loading

Xenorith commented Jun 1, 2023

jenoudet Jun 1, 2023

Xenorith Jun 1, 2023

jenoudet Jun 1, 2023

jenoudet Jun 1, 2023

Xenorith Jun 1, 2023

Xenorith Jun 1, 2023

jenoudet Jun 1, 2023

jenoudet Jun 1, 2023

jenoudet Jun 1, 2023

Xenorith Jun 1, 2023

jenoudet Jun 1, 2023

jenoudet Jun 1, 2023

jenoudet Jun 1, 2023

jenoudet Jun 1, 2023 •

edited

Loading

Xenorith Jun 1, 2023

jenoudet Jun 1, 2023

Xenorith commented Jun 2, 2023

alluxio-bot commented Jun 2, 2023

Xenorith commented Jun 2, 2023

		cmd.Flags().BoolVar(&c.DebugMode, "attachDebug", false, "True to attach debug opts")
		cmd.Flags().StringVar(&c.InlineJavaOpts, "javaOpts", "", `Java options to apply, ex. "-Dkey=value"`)


		var serviceRegistry = map[string]*Service{}

		func RegisterService(p Service) Service {

Add CLI as a golang project #17532

Add CLI as a golang project #17532

Conversation

Xenorith commented Jun 1, 2023 • edited Loading

Xenorith commented Jun 1, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jenoudet Jun 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Xenorith commented Jun 2, 2023

alluxio-bot commented Jun 2, 2023

Xenorith commented Jun 2, 2023

Xenorith commented Jun 1, 2023 •

edited

Loading

jenoudet Jun 1, 2023 •

edited

Loading