From 37d9f94b06ff367a3514b58ec6f0e4d4d0116030 Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Thu, 11 Nov 2021 13:50:36 -0500 Subject: [PATCH] 17840: Deduplicate flag-parsing code. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- cmd/arvados-client/container_gateway.go | 31 +++--------- lib/boot/cmd.go | 17 ++++--- lib/cloud/cloudtest/cmd.go | 12 ++--- lib/cmd/parseflags.go | 50 +++++++++++++++++++ lib/config/cmd.go | 29 +++-------- lib/config/cmd_test.go | 2 +- lib/costanalyzer/cmd.go | 9 ++-- lib/costanalyzer/costanalyzer.go | 46 +++++++---------- lib/costanalyzer/costanalyzer_test.go | 29 +++++------ lib/crunchrun/crunchrun.go | 10 ++-- lib/deduplicationreport/report.go | 44 +++++++--------- lib/diagnostics/cmd.go | 14 ++---- lib/install/deps.go | 12 ++--- lib/install/init.go | 11 +--- lib/mount/command.go | 13 +++-- lib/mount/command_test.go | 2 +- lib/recovercollection/cmd.go | 18 +++---- lib/service/cmd.go | 11 +--- services/arv-git-httpd/main.go | 16 ++---- .../crunch-dispatch-local.go | 32 ++++-------- .../crunch-dispatch-slurm.go | 12 ++--- services/crunch-dispatch-slurm/usage.go | 5 +- services/crunchstat/crunchstat.go | 23 +++------ services/keep-balance/main.go | 11 ++-- services/keep-web/main.go | 23 +++------ services/keepproxy/keepproxy.go | 11 ++-- services/keepstore/command.go | 25 +++++----- tools/keep-block-check/keep-block-check.go | 38 ++++++++------ .../keep-block-check/keep-block-check_test.go | 33 +++++++----- tools/keep-exercise/keep-exercise.go | 12 ++--- tools/keep-rsync/keep-rsync.go | 11 ++-- tools/sync-groups/sync-groups.go | 15 +++--- 32 files changed, 274 insertions(+), 353 deletions(-) create mode 100644 lib/cmd/parseflags.go diff --git a/cmd/arvados-client/container_gateway.go b/cmd/arvados-client/container_gateway.go index 5359e00c66..aca6c5b797 100644 --- a/cmd/arvados-client/container_gateway.go +++ b/cmd/arvados-client/container_gateway.go @@ -17,6 +17,7 @@ import ( "strings" "syscall" + "git.arvados.org/arvados.git/lib/cmd" "git.arvados.org/arvados.git/lib/controller/rpc" "git.arvados.org/arvados.git/sdk/go/arvados" ) @@ -27,26 +28,11 @@ type shellCommand struct{} func (shellCommand) RunCommand(prog string, args []string, stdin io.Reader, stdout, stderr io.Writer) int { f := flag.NewFlagSet(prog, flag.ContinueOnError) - f.SetOutput(stderr) - f.Usage = func() { - _, prog := filepath.Split(prog) - fmt.Fprint(stderr, prog+`: open an interactive shell on a running container. - -Usage: `+prog+` [options] [username@]container-uuid [ssh-options] [remote-command [args...]] - -Options: -`) - f.PrintDefaults() - } detachKeys := f.String("detach-keys", "ctrl-],ctrl-]", "set detach key sequence, as in docker-attach(1)") - err := f.Parse(args) - if err != nil { - fmt.Fprintln(stderr, err) - return 2 - } - - if f.NArg() < 1 { - f.Usage() + if ok, code := cmd.ParseFlags(f, prog, args, "[username@]container-uuid [ssh-options] [remote-command [args...]]", stderr); !ok { + return code + } else if f.NArg() < 1 { + fmt.Fprintf(stderr, "missing required argument: container-uuid (try -help)\n") return 2 } target := f.Args()[0] @@ -127,11 +113,10 @@ Options: } probeOnly := f.Bool("probe-only", false, "do not transfer IO, just setup tunnel, print target UUID, and exit") detachKeys := f.String("detach-keys", "", "set detach key sequence, as in docker-attach(1)") - if err := f.Parse(args); err != nil { - fmt.Fprintln(stderr, err) - return 2 + if ok, code := cmd.ParseFlags(f, prog, args, "[username@]container-uuid", stderr); !ok { + return code } else if f.NArg() != 1 { - f.Usage() + fmt.Fprintf(stderr, "missing required argument: [username@]container-uuid\n") return 2 } targetUUID := f.Args()[0] diff --git a/lib/boot/cmd.go b/lib/boot/cmd.go index b6bbb97995..96241d24b9 100644 --- a/lib/boot/cmd.go +++ b/lib/boot/cmd.go @@ -30,6 +30,7 @@ type supervisedTask interface { } var errNeedConfigReload = errors.New("config changed, restart needed") +var errParseFlags = errors.New("error parsing command line arguments") type bootCommand struct{} @@ -40,6 +41,8 @@ func (bcmd bootCommand) RunCommand(prog string, args []string, stdin io.Reader, err := bcmd.run(ctx, prog, args, stdin, stdout, stderr) if err == errNeedConfigReload { continue + } else if err == errParseFlags { + return 2 } else if err != nil { logger.WithError(err).Info("exiting") return 1 @@ -58,7 +61,6 @@ func (bcmd bootCommand) run(ctx context.Context, prog string, args []string, std } flags := flag.NewFlagSet(prog, flag.ContinueOnError) - flags.SetOutput(stderr) loader := config.NewLoader(stdin, super.logger) loader.SetupFlags(flags) versionFlag := flags.Bool("version", false, "Write version information to stdout and exit 0") @@ -70,13 +72,12 @@ func (bcmd bootCommand) run(ctx context.Context, prog string, args []string, std flags.BoolVar(&super.OwnTemporaryDatabase, "own-temporary-database", false, "bring up a postgres server and create a temporary database") timeout := flags.Duration("timeout", 0, "maximum time to wait for cluster to be ready") shutdown := flags.Bool("shutdown", false, "shut down when the cluster becomes ready") - err := flags.Parse(args) - if err == flag.ErrHelp { - return nil - } else if err != nil { - return err - } else if flags.NArg() != 0 { - return fmt.Errorf("unrecognized command line arguments: %v", flags.Args()) + if ok, code := cmd.ParseFlags(flags, prog, args, "", stderr); !ok { + if code == 0 { + return nil + } else { + return errParseFlags + } } else if *versionFlag { cmd.Version.RunCommand(prog, args, stdin, stdout, stderr) return nil diff --git a/lib/cloud/cloudtest/cmd.go b/lib/cloud/cloudtest/cmd.go index 3c4b560c97..0ec79e1175 100644 --- a/lib/cloud/cloudtest/cmd.go +++ b/lib/cloud/cloudtest/cmd.go @@ -13,6 +13,7 @@ import ( "os" "git.arvados.org/arvados.git/lib/cloud" + "git.arvados.org/arvados.git/lib/cmd" "git.arvados.org/arvados.git/lib/config" "git.arvados.org/arvados.git/lib/dispatchcloud" "git.arvados.org/arvados.git/sdk/go/arvados" @@ -41,15 +42,8 @@ func (command) RunCommand(prog string, args []string, stdin io.Reader, stdout, s destroyExisting := flags.Bool("destroy-existing", false, "Destroy any existing instances tagged with our InstanceSetID, instead of erroring out") shellCommand := flags.String("command", "", "Run an interactive shell command on the test instance when it boots") pauseBeforeDestroy := flags.Bool("pause-before-destroy", false, "Prompt and wait before destroying the test instance") - err = flags.Parse(args) - if err == flag.ErrHelp { - err = nil - return 0 - } else if err != nil { - return 2 - } else if flags.NArg() != 0 { - err = fmt.Errorf("unrecognized command line arguments: %v", flags.Args()) - return 2 + if ok, code := cmd.ParseFlags(flags, prog, args, "", stderr); !ok { + return code } logger := ctxlog.New(stderr, "text", "info") defer func() { diff --git a/lib/cmd/parseflags.go b/lib/cmd/parseflags.go new file mode 100644 index 0000000000..3e872fcd11 --- /dev/null +++ b/lib/cmd/parseflags.go @@ -0,0 +1,50 @@ +// Copyright (C) The Arvados Authors. All rights reserved. +// +// SPDX-License-Identifier: Apache-2.0 + +package cmd + +import ( + "flag" + "fmt" + "io" +) + +// ParseFlags calls f.Parse(args) and prints appropriate error/help +// messages to stderr. +// +// The positional argument is "" if no positional arguments are +// accepted, otherwise a string to print with the usage message, +// "Usage: {prog} [options] {positional}". +// +// The first return value, ok, is true if the program should continue +// running normally, or false if it should exit now. +// +// If ok is false, the second return value is an appropriate exit +// code: 0 if "-help" was given, 2 if there was a usage error. +func ParseFlags(f FlagSet, prog string, args []string, positional string, stderr io.Writer) (ok bool, exitCode int) { + f.Init(prog, flag.ContinueOnError) + f.SetOutput(io.Discard) + err := f.Parse(args) + switch err { + case nil: + if f.NArg() > 0 && positional == "" { + fmt.Fprintf(stderr, "unrecognized command line arguments: %v (try -help)\n", f.Args()) + return false, 2 + } + return true, 0 + case flag.ErrHelp: + if f, ok := f.(*flag.FlagSet); ok && f.Usage != nil { + f.SetOutput(stderr) + f.Usage() + } else { + fmt.Fprintf(stderr, "Usage: %s [options] %s\n", prog, positional) + f.SetOutput(stderr) + f.PrintDefaults() + } + return false, 0 + default: + fmt.Fprintf(stderr, "error parsing command line arguments: %s (try -help)\n", err) + return false, 2 + } +} diff --git a/lib/config/cmd.go b/lib/config/cmd.go index c852b0b545..eeab6ac8cd 100644 --- a/lib/config/cmd.go +++ b/lib/config/cmd.go @@ -12,6 +12,7 @@ import ( "os" "os/exec" + "git.arvados.org/arvados.git/lib/cmd" "git.arvados.org/arvados.git/sdk/go/ctxlog" "github.com/ghodss/yaml" "github.com/sirupsen/logrus" @@ -35,20 +36,11 @@ func (dumpCommand) RunCommand(prog string, args []string, stdin io.Reader, stdou } flags := flag.NewFlagSet("", flag.ContinueOnError) - flags.SetOutput(stderr) loader.SetupFlags(flags) - err = flags.Parse(args) - if err == flag.ErrHelp { - err = nil - return 0 - } else if err != nil { - return 2 - } else if flags.NArg() != 0 { - err = fmt.Errorf("unrecognized command line arguments: %v", flags.Args()) - return 2 + if ok, code := cmd.ParseFlags(flags, prog, args, "", stderr); !ok { + return code } - cfg, err := loader.Load() if err != nil { return 1 @@ -85,20 +77,11 @@ func (checkCommand) RunCommand(prog string, args []string, stdin io.Reader, stdo Logger: logger, } - flags := flag.NewFlagSet("", flag.ContinueOnError) - flags.SetOutput(stderr) + flags := flag.NewFlagSet(prog, flag.ContinueOnError) loader.SetupFlags(flags) strict := flags.Bool("strict", true, "Strict validation of configuration file (warnings result in non-zero exit code)") - - err = flags.Parse(args) - if err == flag.ErrHelp { - err = nil - return 0 - } else if err != nil { - return 2 - } else if flags.NArg() != 0 { - err = fmt.Errorf("unrecognized command line arguments: %v", flags.Args()) - return 2 + if ok, code := cmd.ParseFlags(flags, prog, args, "", stderr); !ok { + return code } // Load the config twice -- once without loading deprecated diff --git a/lib/config/cmd_test.go b/lib/config/cmd_test.go index 241f376834..a5cc28b80c 100644 --- a/lib/config/cmd_test.go +++ b/lib/config/cmd_test.go @@ -34,7 +34,7 @@ func (s *CommandSuite) TestDump_BadArg(c *check.C) { var stderr bytes.Buffer code := DumpCommand.RunCommand("arvados config-dump", []string{"-badarg"}, bytes.NewBuffer(nil), bytes.NewBuffer(nil), &stderr) c.Check(code, check.Equals, 2) - c.Check(stderr.String(), check.Matches, `(?ms)flag provided but not defined: -badarg\nUsage:\n.*`) + c.Check(stderr.String(), check.Equals, "error parsing command line arguments: flag provided but not defined: -badarg (try -help)\n") } func (s *CommandSuite) TestDump_EmptyInput(c *check.C) { diff --git a/lib/costanalyzer/cmd.go b/lib/costanalyzer/cmd.go index 6065ad2c0b..f2a7af4933 100644 --- a/lib/costanalyzer/cmd.go +++ b/lib/costanalyzer/cmd.go @@ -27,13 +27,10 @@ func (c command) RunCommand(prog string, args []string, stdin io.Reader, stdout, var err error logger := ctxlog.New(stderr, "text", "info") logger.SetFormatter(cmd.NoPrefixFormatter{}) - defer func() { - if err != nil { - logger.Error("\n" + err.Error()) - } - }() exitcode, err := c.costAnalyzer(prog, args, logger, stdout, stderr) - + if err != nil { + logger.Error("\n" + err.Error()) + } return exitcode } diff --git a/lib/costanalyzer/costanalyzer.go b/lib/costanalyzer/costanalyzer.go index 4a48db1a8b..a3673c9794 100644 --- a/lib/costanalyzer/costanalyzer.go +++ b/lib/costanalyzer/costanalyzer.go @@ -17,6 +17,7 @@ import ( "strings" "time" + "git.arvados.org/arvados.git/lib/cmd" "git.arvados.org/arvados.git/sdk/go/arvados" "git.arvados.org/arvados.git/sdk/go/arvadosclient" "git.arvados.org/arvados.git/sdk/go/keepclient" @@ -62,10 +63,9 @@ func (i *arrayFlags) Set(value string) error { return nil } -func (c *command) parseFlags(prog string, args []string, logger *logrus.Logger, stderr io.Writer) (exitCode int, err error) { +func (c *command) parseFlags(prog string, args []string, logger *logrus.Logger, stderr io.Writer) (ok bool, exitCode int) { var beginStr, endStr string flags := flag.NewFlagSet("", flag.ContinueOnError) - flags.SetOutput(stderr) flags.Usage = func() { fmt.Fprintf(flags.Output(), ` Usage: @@ -135,22 +135,14 @@ Options: flags.StringVar(&beginStr, "begin", "", fmt.Sprintf("timestamp `begin` for date range operation (format: %s)", timestampFormat)) flags.StringVar(&endStr, "end", "", fmt.Sprintf("timestamp `end` for date range operation (format: %s)", timestampFormat)) flags.BoolVar(&c.cache, "cache", true, "create and use a local disk cache of Arvados objects") - err = flags.Parse(args) - if err == flag.ErrHelp { - err = nil - exitCode = 1 - return - } else if err != nil { - exitCode = 2 - return + if ok, code := cmd.ParseFlags(flags, prog, args, "[uuid ...]", stderr); !ok { + return false, code } c.uuids = flags.Args() if (len(beginStr) != 0 && len(endStr) == 0) || (len(beginStr) == 0 && len(endStr) != 0) { - flags.Usage() - err = fmt.Errorf("When specifying a date range, both begin and end must be specified") - exitCode = 2 - return + fmt.Fprintf(stderr, "When specifying a date range, both begin and end must be specified (try -help)\n") + return false, 2 } if len(beginStr) != 0 { @@ -158,30 +150,26 @@ Options: c.begin, errB = time.Parse(timestampFormat, beginStr) c.end, errE = time.Parse(timestampFormat, endStr) if (errB != nil) || (errE != nil) { - flags.Usage() - err = fmt.Errorf("When specifying a date range, both begin and end must be of the format %s %+v, %+v", timestampFormat, errB, errE) - exitCode = 2 - return + fmt.Fprintf(stderr, "When specifying a date range, both begin and end must be of the format %s %+v, %+v\n", timestampFormat, errB, errE) + return false, 2 } } if (len(c.uuids) < 1) && (len(beginStr) == 0) { - flags.Usage() - err = fmt.Errorf("error: no uuid(s) provided") - exitCode = 2 - return + fmt.Fprintf(stderr, "error: no uuid(s) provided (try -help)\n") + return false, 2 } lvl, err := logrus.ParseLevel(*loglevel) if err != nil { - exitCode = 2 - return + fmt.Fprintf(stderr, "invalid argument to -log-level: %s\n", err) + return false, 2 } logger.SetLevel(lvl) if !c.cache { logger.Debug("Caching disabled") } - return + return true, 0 } func ensureDirectory(logger *logrus.Logger, dir string) (err error) { @@ -526,9 +514,9 @@ func generateCrInfo(logger *logrus.Logger, uuid string, arv *arvadosclient.Arvad } func (c *command) costAnalyzer(prog string, args []string, logger *logrus.Logger, stdout, stderr io.Writer) (exitcode int, err error) { - exitcode, err = c.parseFlags(prog, args, logger, stderr) - - if exitcode != 0 { + var ok bool + ok, exitcode = c.parseFlags(prog, args, logger, stderr) + if !ok { return } if c.resultsDir != "" { @@ -610,7 +598,7 @@ func (c *command) costAnalyzer(prog string, args []string, logger *logrus.Logger cost[k] = v } } else if strings.Contains(uuid, "-xvhdp-") || strings.Contains(uuid, "-4zz18-") { - // This is a container request + // This is a container request or collection var crInfo map[string]consumption crInfo, err = generateCrInfo(logger, uuid, arv, ac, kc, c.resultsDir, c.cache) if err != nil { diff --git a/lib/costanalyzer/costanalyzer_test.go b/lib/costanalyzer/costanalyzer_test.go index 2975e3b3de..b78b288ab0 100644 --- a/lib/costanalyzer/costanalyzer_test.go +++ b/lib/costanalyzer/costanalyzer_test.go @@ -152,7 +152,7 @@ func createNodeJSON(c *check.C, arv *arvadosclient.ArvadosClient, ac *arvados.Cl func (*Suite) TestUsage(c *check.C) { var stdout, stderr bytes.Buffer exitcode := Command.RunCommand("costanalyzer.test", []string{"-help", "-log-level=debug"}, &bytes.Buffer{}, &stdout, &stderr) - c.Check(exitcode, check.Equals, 1) + c.Check(exitcode, check.Equals, 0) c.Check(stdout.String(), check.Equals, "") c.Check(stderr.String(), check.Matches, `(?ms).*Usage:.*`) } @@ -205,19 +205,23 @@ func (*Suite) TestContainerRequestUUID(c *check.C) { func (*Suite) TestCollectionUUID(c *check.C) { var stdout, stderr bytes.Buffer - resultsDir := c.MkDir() - // Run costanalyzer with 1 collection uuid, without 'container_request' property - exitcode := Command.RunCommand("costanalyzer.test", []string{"-output", resultsDir, arvadostest.FooCollection}, &bytes.Buffer{}, &stdout, &stderr) - c.Check(exitcode, check.Equals, 2) - c.Assert(stderr.String(), check.Matches, "(?ms).*does not have a 'container_request' property.*") - // Update the collection, attach a 'container_request' property + // Create a collection with no container_request property ac := arvados.NewClientFromEnv() var coll arvados.Collection + err := ac.RequestAndDecode(&coll, "POST", "arvados/v1/collections", nil, nil) + c.Assert(err, check.IsNil) - // Update collection record - err := ac.RequestAndDecode(&coll, "PUT", "arvados/v1/collections/"+arvadostest.FooCollection, nil, map[string]interface{}{ + exitcode := Command.RunCommand("costanalyzer.test", []string{"-output", resultsDir, coll.UUID}, &bytes.Buffer{}, &stdout, &stderr) + c.Check(exitcode, check.Equals, 2) + c.Assert(stderr.String(), check.Matches, "(?ms).*does not have a 'container_request' property.*") + + stdout.Truncate(0) + stderr.Truncate(0) + + // Add a container_request property + err = ac.RequestAndDecode(&coll, "PATCH", "arvados/v1/collections/"+coll.UUID, nil, map[string]interface{}{ "collection": map[string]interface{}{ "properties": map[string]interface{}{ "container_request": arvadostest.CompletedContainerRequestUUID, @@ -226,12 +230,9 @@ func (*Suite) TestCollectionUUID(c *check.C) { }) c.Assert(err, check.IsNil) - stdout.Truncate(0) - stderr.Truncate(0) - - // Run costanalyzer with 1 collection uuid + // Re-run costanalyzer on the updated collection resultsDir = c.MkDir() - exitcode = Command.RunCommand("costanalyzer.test", []string{"-output", resultsDir, arvadostest.FooCollection}, &bytes.Buffer{}, &stdout, &stderr) + exitcode = Command.RunCommand("costanalyzer.test", []string{"-output", resultsDir, coll.UUID}, &bytes.Buffer{}, &stdout, &stderr) c.Check(exitcode, check.Equals, 0) c.Assert(stderr.String(), check.Matches, "(?ms).*supplied uuids in .*") diff --git a/lib/crunchrun/crunchrun.go b/lib/crunchrun/crunchrun.go index a5e69387ec..f9bf3df9d7 100644 --- a/lib/crunchrun/crunchrun.go +++ b/lib/crunchrun/crunchrun.go @@ -1694,14 +1694,10 @@ func (command) RunCommand(prog string, args []string, stdin io.Reader, stdout, s ignoreDetachFlag = true } - if err := flags.Parse(args); err == flag.ErrHelp { - return 0 - } else if err != nil { - log.Print(err) - return 1 + if ok, code := cmd.ParseFlags(flags, prog, args, "container-uuid", stderr); !ok { + return code } else if flags.NArg() != 1 { - fmt.Fprintf(flags.Output(), "Usage: %s [options] containerUUID\n\nOptions:\n", prog) - flags.PrintDefaults() + fmt.Fprintf(stderr, "missing required argument: container-uuid (try -help)\n") return 2 } diff --git a/lib/deduplicationreport/report.go b/lib/deduplicationreport/report.go index bb3405a493..2f9521c65d 100644 --- a/lib/deduplicationreport/report.go +++ b/lib/deduplicationreport/report.go @@ -10,6 +10,7 @@ import ( "io" "strings" + "git.arvados.org/arvados.git/lib/cmd" "git.arvados.org/arvados.git/sdk/go/arvados" "git.arvados.org/arvados.git/sdk/go/arvadosclient" "git.arvados.org/arvados.git/sdk/go/manifest" @@ -29,16 +30,17 @@ func deDuplicate(inputs []string) (trimmed []string) { return } -func parseFlags(prog string, args []string, logger *logrus.Logger, stderr io.Writer) ([]string, error) { - flags := flag.NewFlagSet("", flag.ContinueOnError) - flags.SetOutput(stderr) +// parseFlags returns either some inputs to process, or (if there are +// no inputs to process) a nil slice and a suitable exit code. +func parseFlags(prog string, args []string, logger *logrus.Logger, stderr io.Writer) (inputs []string, exitcode int) { + flags := flag.NewFlagSet(prog, flag.ContinueOnError) flags.Usage = func() { fmt.Fprintf(flags.Output(), ` Usage: %s [options ...] ... - %s [options ...] , \ - , ... + %s [options ...] , \ + , ... This program analyzes the overlap in blocks used by 2 or more collections. It prints a deduplication report that shows the nominal space used by the @@ -67,28 +69,24 @@ Options: flags.PrintDefaults() } loglevel := flags.String("log-level", "info", "logging level (debug, info, ...)") - err := flags.Parse(args) - if err == flag.ErrHelp { - return nil, err - } else if err != nil { - return nil, err + if ok, code := cmd.ParseFlags(flags, prog, args, "collection-uuid [...]", stderr); !ok { + return nil, code } - inputs := flags.Args() - - inputs = deDuplicate(inputs) + inputs = deDuplicate(flags.Args()) if len(inputs) < 1 { - err = fmt.Errorf("Error: no collections provided") - return inputs, err + fmt.Fprintf(stderr, "Error: no collections provided\n") + return nil, 2 } lvl, err := logrus.ParseLevel(*loglevel) if err != nil { - return inputs, err + fmt.Fprintf(stderr, "Error: cannot parse log level: %s\n", err) + return nil, 2 } logger.SetLevel(lvl) - return inputs, err + return inputs, 0 } func blockList(collection arvados.Collection) (blocks map[string]int) { @@ -103,14 +101,10 @@ func blockList(collection arvados.Collection) (blocks map[string]int) { func report(prog string, args []string, logger *logrus.Logger, stdout, stderr io.Writer) (exitcode int) { var inputs []string - var err error - - inputs, err = parseFlags(prog, args, logger, stderr) - if err == flag.ErrHelp { - return 0 - } else if err != nil { - logger.Error(err.Error()) - return 2 + + inputs, exitcode = parseFlags(prog, args, logger, stderr) + if inputs == nil { + return } // Arvados Client setup diff --git a/lib/diagnostics/cmd.go b/lib/diagnostics/cmd.go index 8663c6ee5f..71fe1c5dc6 100644 --- a/lib/diagnostics/cmd.go +++ b/lib/diagnostics/cmd.go @@ -17,6 +17,7 @@ import ( "strings" "time" + "git.arvados.org/arvados.git/lib/cmd" "git.arvados.org/arvados.git/sdk/go/arvados" "git.arvados.org/arvados.git/sdk/go/ctxlog" "github.com/sirupsen/logrus" @@ -24,7 +25,7 @@ import ( type Command struct{} -func (cmd Command) RunCommand(prog string, args []string, stdin io.Reader, stdout, stderr io.Writer) int { +func (Command) RunCommand(prog string, args []string, stdin io.Reader, stdout, stderr io.Writer) int { var diag diagnoser f := flag.NewFlagSet(prog, flag.ContinueOnError) f.StringVar(&diag.projectName, "project-name", "scratch area for diagnostics", "name of project to find/create in home project and use for temporary/test objects") @@ -33,15 +34,8 @@ func (cmd Command) RunCommand(prog string, args []string, stdin io.Reader, stdou f.BoolVar(&diag.checkExternal, "external-client", false, "check that this host is considered an \"external\" client") f.IntVar(&diag.priority, "priority", 500, "priority for test container (1..1000, or 0 to skip)") f.DurationVar(&diag.timeout, "timeout", 10*time.Second, "timeout for http requests") - err := f.Parse(args) - if err == flag.ErrHelp { - return 0 - } else if err != nil { - fmt.Fprintln(stderr, err) - return 2 - } else if f.NArg() != 0 { - fmt.Fprintf(stderr, "unrecognized command line arguments: %v\n", f.Args()) - return 2 + if ok, code := cmd.ParseFlags(f, prog, args, "", stderr); !ok { + return code } diag.logger = ctxlog.New(stdout, "text", diag.logLevel) diag.logger.SetFormatter(&logrus.TextFormatter{DisableTimestamp: true, DisableLevelTruncation: true, PadLevelText: true}) diff --git a/lib/install/deps.go b/lib/install/deps.go index ff00ee1e35..ca4b681475 100644 --- a/lib/install/deps.go +++ b/lib/install/deps.go @@ -57,17 +57,11 @@ func (inst *installCommand) RunCommand(prog string, args []string, stdin io.Read flags.StringVar(&inst.SourcePath, "source", "/arvados", "source tree location (required for -type=package)") flags.StringVar(&inst.PackageVersion, "package-version", "0.0.0", "version string to embed in executable files") flags.BoolVar(&inst.EatMyData, "eatmydata", false, "use eatmydata to speed up install") - err = flags.Parse(args) - if err == flag.ErrHelp { - err = nil - return 0 - } else if err != nil { - return 2 + + if ok, code := cmd.ParseFlags(flags, prog, args, "", stderr); !ok { + return code } else if *versionFlag { return cmd.Version.RunCommand(prog, args, stdin, stdout, stderr) - } else if len(flags.Args()) > 0 { - err = fmt.Errorf("unrecognized command line arguments: %v", flags.Args()) - return 2 } var dev, test, prod, pkg bool diff --git a/lib/install/init.go b/lib/install/init.go index 8b2fbfdd02..1d063506b8 100644 --- a/lib/install/init.go +++ b/lib/install/init.go @@ -59,17 +59,10 @@ func (initcmd *initCommand) RunCommand(prog string, args []string, stdin io.Read versionFlag := flags.Bool("version", false, "Write version information to stdout and exit 0") flags.StringVar(&initcmd.ClusterID, "cluster-id", "", "cluster `id`, like x1234 for a dev cluster") flags.StringVar(&initcmd.Domain, "domain", hostname, "cluster public DNS `name`, like x1234.arvadosapi.com") - err = flags.Parse(args) - if err == flag.ErrHelp { - err = nil - return 0 - } else if err != nil { - return 2 + if ok, code := cmd.ParseFlags(flags, prog, args, "", stderr); !ok { + return code } else if *versionFlag { return cmd.Version.RunCommand(prog, args, stdin, stdout, stderr) - } else if flags.NArg() != 0 { - err = fmt.Errorf("unrecognized command line arguments: %v", flags.Args()) - return 2 } else if !regexp.MustCompile(`^[a-z][a-z0-9]{4}`).MatchString(initcmd.ClusterID) { err = fmt.Errorf("cluster ID %q is invalid; must be an ASCII letter followed by 4 alphanumerics (try -help)", initcmd.ClusterID) return 1 diff --git a/lib/mount/command.go b/lib/mount/command.go index d6d1ae0308..f88d977c4c 100644 --- a/lib/mount/command.go +++ b/lib/mount/command.go @@ -14,15 +14,16 @@ import ( _ "net/http/pprof" "os" + "git.arvados.org/arvados.git/lib/cmd" "git.arvados.org/arvados.git/sdk/go/arvados" "git.arvados.org/arvados.git/sdk/go/arvadosclient" "git.arvados.org/arvados.git/sdk/go/keepclient" "github.com/arvados/cgofuse/fuse" ) -var Command = &cmd{} +var Command = &mountCommand{} -type cmd struct { +type mountCommand struct { // ready, if non-nil, will be closed when the mount is // initialized. If ready is non-nil, it RunCommand() should // not be called more than once, or when ready is already @@ -37,17 +38,15 @@ type cmd struct { // // The "-d" fuse option (and perhaps other features) ignores the // stderr argument and prints to os.Stderr instead. -func (c *cmd) RunCommand(prog string, args []string, stdin io.Reader, stdout, stderr io.Writer) int { +func (c *mountCommand) RunCommand(prog string, args []string, stdin io.Reader, stdout, stderr io.Writer) int { logger := log.New(stderr, prog+" ", 0) flags := flag.NewFlagSet(prog, flag.ContinueOnError) ro := flags.Bool("ro", false, "read-only") experimental := flags.Bool("experimental", false, "acknowledge this is an experimental command, and should not be used in production (required)") blockCache := flags.Int("block-cache", 4, "read cache size (number of 64MiB blocks)") pprof := flags.String("pprof", "", "serve Go profile data at `[addr]:port`") - err := flags.Parse(args) - if err != nil { - logger.Print(err) - return 2 + if ok, code := cmd.ParseFlags(flags, prog, args, "[FUSE mount options]", stderr); !ok { + return code } if !*experimental { logger.Printf("error: experimental command %q used without --experimental flag", prog) diff --git a/lib/mount/command_test.go b/lib/mount/command_test.go index 980b7d2ae3..44eb61e7f9 100644 --- a/lib/mount/command_test.go +++ b/lib/mount/command_test.go @@ -36,7 +36,7 @@ func (s *CmdSuite) TestMount(c *check.C) { stdin := bytes.NewBufferString("stdin") stdout := bytes.NewBuffer(nil) stderr := bytes.NewBuffer(nil) - mountCmd := cmd{ready: make(chan struct{})} + mountCmd := mountCommand{ready: make(chan struct{})} ready := false go func() { exited <- mountCmd.RunCommand("test mount", []string{"--experimental", s.mnt}, stdin, stdout, stderr) diff --git a/lib/recovercollection/cmd.go b/lib/recovercollection/cmd.go index da466c31ca..5038e4788a 100644 --- a/lib/recovercollection/cmd.go +++ b/lib/recovercollection/cmd.go @@ -15,6 +15,7 @@ import ( "sync" "time" + "git.arvados.org/arvados.git/lib/cmd" "git.arvados.org/arvados.git/lib/config" "git.arvados.org/arvados.git/sdk/go/arvados" "git.arvados.org/arvados.git/sdk/go/ctxlog" @@ -38,8 +39,7 @@ func (command) RunCommand(prog string, args []string, stdin io.Reader, stdout, s loader := config.NewLoader(stdin, logger) loader.SkipLegacy = true - flags := flag.NewFlagSet("", flag.ContinueOnError) - flags.SetOutput(stderr) + flags := flag.NewFlagSet(prog, flag.ContinueOnError) flags.Usage = func() { fmt.Fprintf(flags.Output(), `Usage: %s [options ...] { /path/to/manifest.txt | log-or-collection-uuid } [...] @@ -79,16 +79,10 @@ Options: } loader.SetupFlags(flags) loglevel := flags.String("log-level", "info", "logging level (debug, info, ...)") - err = flags.Parse(args) - if err == flag.ErrHelp { - err = nil - return 0 - } else if err != nil { - return 2 - } - - if len(flags.Args()) == 0 { - flags.Usage() + if ok, code := cmd.ParseFlags(flags, prog, args, "source [...]", stderr); !ok { + return code + } else if flags.NArg() == 0 { + fmt.Fprintf(stderr, "missing required arguments (try -help)\n") return 2 } diff --git a/lib/service/cmd.go b/lib/service/cmd.go index 268aba7516..880799b348 100644 --- a/lib/service/cmd.go +++ b/lib/service/cmd.go @@ -72,15 +72,8 @@ func (c *command) RunCommand(prog string, args []string, stdin io.Reader, stdout loader.SetupFlags(flags) versionFlag := flags.Bool("version", false, "Write version information to stdout and exit 0") pprofAddr := flags.String("pprof", "", "Serve Go profile data at `[addr]:port`") - err = flags.Parse(args) - if err == flag.ErrHelp { - err = nil - return 0 - } else if err != nil { - return 2 - } else if flags.NArg() != 0 { - err = fmt.Errorf("unrecognized command line arguments: %v", flags.Args()) - return 2 + if ok, code := cmd.ParseFlags(flags, prog, args, "", stderr); !ok { + return code } else if *versionFlag { return cmd.Version.RunCommand(prog, args, stdin, stdout, stderr) } diff --git a/services/arv-git-httpd/main.go b/services/arv-git-httpd/main.go index af63b32ab3..b926ac2735 100644 --- a/services/arv-git-httpd/main.go +++ b/services/arv-git-httpd/main.go @@ -9,6 +9,7 @@ import ( "fmt" "os" + "git.arvados.org/arvados.git/lib/cmd" "git.arvados.org/arvados.git/lib/config" "github.com/coreos/go-systemd/daemon" "github.com/ghodss/yaml" @@ -31,18 +32,9 @@ func main() { getVersion := flags.Bool("version", false, "print version information and exit.") args := loader.MungeLegacyConfigArgs(logger, os.Args[1:], "-legacy-git-httpd-config") - err := flags.Parse(args) - if err == flag.ErrHelp { - return - } else if err != nil { - logger.Error(err) - os.Exit(2) - } else if flags.NArg() != 0 { - logger.Errorf("unrecognized command line arguments: %v", flags.Args()) - os.Exit(2) - } - - if *getVersion { + if ok, code := cmd.ParseFlags(flags, os.Args[0], args, "", os.Stderr); !ok { + os.Exit(code) + } else if *getVersion { fmt.Printf("arv-git-httpd %s\n", version) return } diff --git a/services/crunch-dispatch-local/crunch-dispatch-local.go b/services/crunch-dispatch-local/crunch-dispatch-local.go index 159ee69b12..be42252762 100644 --- a/services/crunch-dispatch-local/crunch-dispatch-local.go +++ b/services/crunch-dispatch-local/crunch-dispatch-local.go @@ -17,6 +17,7 @@ import ( "syscall" "time" + "git.arvados.org/arvados.git/lib/cmd" "git.arvados.org/arvados.git/lib/config" "git.arvados.org/arvados.git/sdk/go/arvados" "git.arvados.org/arvados.git/sdk/go/arvadosclient" @@ -26,13 +27,6 @@ import ( var version = "dev" -func main() { - err := doMain() - if err != nil { - logrus.Fatalf("%q", err) - } -} - var ( runningCmds map[string]*exec.Cmd runningCmdsMutex sync.Mutex @@ -40,7 +34,7 @@ var ( crunchRunCommand *string ) -func doMain() error { +func main() { logger := logrus.StandardLogger() if os.Getenv("DEBUG") != "" { logger.SetLevel(logrus.DebugLevel) @@ -66,27 +60,22 @@ func doMain() error { false, "Print version information and exit.") - // Parse args; omit the first arg which is the command name - err := flags.Parse(os.Args[1:]) - if err == flag.ErrHelp { - return nil - } else if err != nil { - return err - } else if flags.NArg() != 0 { - return fmt.Errorf("unrecognized command line arguments: %v", flags.Args()) + if ok, code := cmd.ParseFlags(flags, os.Args[0], os.Args[1:], "", os.Stderr); !ok { + os.Exit(code) } // Print version information if requested if *getVersion { fmt.Printf("crunch-dispatch-local %s\n", version) - return nil + return } loader := config.NewLoader(nil, logger) cfg, err := loader.Load() cluster, err := cfg.GetCluster("") if err != nil { - return fmt.Errorf("config error: %s", err) + fmt.Fprintf(os.Stderr, "config error: %s\n", err) + os.Exit(1) } logger.Printf("crunch-dispatch-local %s started", version) @@ -116,7 +105,7 @@ func doMain() error { arv, err := arvadosclient.MakeArvadosClient() if err != nil { logger.Errorf("error making Arvados client: %v", err) - return err + os.Exit(1) } arv.Retries = 25 @@ -131,7 +120,8 @@ func doMain() error { err = dispatcher.Run(ctx) if err != nil { - return err + logger.Error(err) + return } c := make(chan os.Signal, 1) @@ -151,8 +141,6 @@ func doMain() error { // Wait for all running crunch jobs to complete / terminate waitGroup.Wait() - - return nil } func startFunc(container arvados.Container, cmd *exec.Cmd) error { diff --git a/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go b/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go index 02493bf4fb..ff1077fae6 100644 --- a/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go +++ b/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go @@ -17,6 +17,7 @@ import ( "strings" "time" + "git.arvados.org/arvados.git/lib/cmd" "git.arvados.org/arvados.git/lib/config" "git.arvados.org/arvados.git/lib/dispatchcloud" "git.arvados.org/arvados.git/sdk/go/arvados" @@ -76,7 +77,7 @@ func (disp *Dispatcher) configure(prog string, args []string) error { if disp.logger == nil { disp.logger = logrus.StandardLogger() } - flags := flag.NewFlagSet(prog, flag.ExitOnError) + flags := flag.NewFlagSet(prog, flag.ContinueOnError) flags.Usage = func() { usage(flags) } loader := config.NewLoader(nil, disp.logger) @@ -92,13 +93,8 @@ func (disp *Dispatcher) configure(prog string, args []string) error { "Print version information and exit.") args = loader.MungeLegacyConfigArgs(disp.logger, args, "-legacy-crunch-dispatch-slurm-config") - err := flags.Parse(args) - if err == flag.ErrHelp { - return nil - } else if err != nil { - return err - } else if flags.NArg() != 0 { - return fmt.Errorf("unrecognized command line arguments: %v", flags.Args()) + if ok, code := cmd.ParseFlags(flags, prog, args, "", os.Stderr); !ok { + os.Exit(code) } // Print version information if requested diff --git a/services/crunch-dispatch-slurm/usage.go b/services/crunch-dispatch-slurm/usage.go index b2d157cbdb..68a2305f74 100644 --- a/services/crunch-dispatch-slurm/usage.go +++ b/services/crunch-dispatch-slurm/usage.go @@ -7,18 +7,17 @@ package main import ( "flag" "fmt" - "os" ) func usage(fs *flag.FlagSet) { - fmt.Fprintf(os.Stderr, ` + fmt.Fprintf(fs.Output(), ` crunch-dispatch-slurm runs queued Arvados containers by submitting SLURM batch jobs. Options: `) fs.PrintDefaults() - fmt.Fprintf(os.Stderr, ` + fmt.Fprintf(fs.Output(), ` For configuration instructions see https://doc.arvados.org/install/crunch2-slurm/install-dispatch.html `) diff --git a/services/crunchstat/crunchstat.go b/services/crunchstat/crunchstat.go index 3ff3912e8c..6383eae545 100644 --- a/services/crunchstat/crunchstat.go +++ b/services/crunchstat/crunchstat.go @@ -16,6 +16,7 @@ import ( "syscall" "time" + "git.arvados.org/arvados.git/lib/cmd" "git.arvados.org/arvados.git/lib/crunchstat" ) @@ -41,23 +42,13 @@ func main() { pollMsec := flags.Int64("poll", 1000, "Reporting interval, in milliseconds") getVersion := flags.Bool("version", false, "Print version information and exit.") - err := flags.Parse(os.Args[1:]) - if err == flag.ErrHelp { - return - } else if err != nil { - reporter.Logger.Print(err) - os.Exit(2) - } - - // Print version information if requested - if *getVersion { + if ok, code := cmd.ParseFlags(flags, os.Args[0], os.Args[1:], "program [args ...]", os.Stderr); !ok { + os.Exit(code) + } else if *getVersion { fmt.Printf("crunchstat %s\n", version) return - } - - if flags.NArg() == 0 { - fmt.Fprintf(flags.Output(), "Usage: %s [options] program [args...]\n\nOptions:\n", os.Args[0]) - flags.PrintDefaults() + } else if flags.NArg() == 0 { + fmt.Fprintf(os.Stderr, "missing required argument: program (try -help)\n") os.Exit(2) } @@ -71,7 +62,7 @@ func main() { reporter.PollPeriod = time.Duration(*pollMsec) * time.Millisecond reporter.Start() - err = runCommand(flags.Args(), reporter.Logger) + err := runCommand(flags.Args(), reporter.Logger) reporter.Stop() if err, ok := err.(*exec.ExitError); ok { diff --git a/services/keep-balance/main.go b/services/keep-balance/main.go index 49cb17dfa2..8a95d389c8 100644 --- a/services/keep-balance/main.go +++ b/services/keep-balance/main.go @@ -13,6 +13,7 @@ import ( _ "net/http/pprof" "os" + "git.arvados.org/arvados.git/lib/cmd" "git.arvados.org/arvados.git/lib/config" "git.arvados.org/arvados.git/lib/service" "git.arvados.org/arvados.git/sdk/go/arvados" @@ -58,14 +59,8 @@ func runCommand(prog string, args []string, stdin io.Reader, stdout, stderr io.W loader.SetupFlags(flags) munged := loader.MungeLegacyConfigArgs(logger, args, "-legacy-keepbalance-config") - err := flags.Parse(munged) - if err == flag.ErrHelp { - return 0 - } else if err != nil { - return 2 - } else if flags.NArg() != 0 { - fmt.Fprintf(stderr, "unrecognized command line arguments: %v", flags.Args()) - return 2 + if ok, code := cmd.ParseFlags(flags, prog, munged, "", stderr); !ok { + return code } if *dumpFlag { diff --git a/services/keep-web/main.go b/services/keep-web/main.go index 4c66165225..aa97a18ad8 100644 --- a/services/keep-web/main.go +++ b/services/keep-web/main.go @@ -10,6 +10,7 @@ import ( "mime" "os" + "git.arvados.org/arvados.git/lib/cmd" "git.arvados.org/arvados.git/lib/config" "git.arvados.org/arvados.git/sdk/go/arvados" "github.com/coreos/go-systemd/daemon" @@ -69,25 +70,18 @@ func configure(logger log.FieldLogger, args []string) (*Config, error) { getVersion := flags.Bool("version", false, "print version information and exit.") + prog := args[0] args = loader.MungeLegacyConfigArgs(logger, args[1:], "-legacy-keepweb-config") - err := flags.Parse(args) - if err == flag.ErrHelp { - return nil, nil - } else if err != nil { - return nil, err - } else if flags.NArg() != 0 { - return nil, fmt.Errorf("unrecognized command line arguments: %v", flags.Args()) - } - - // Print version information if requested - if *getVersion { - fmt.Printf("keep-web %s\n", version) + if ok, code := cmd.ParseFlags(flags, prog, args, "", os.Stderr); !ok { + os.Exit(code) + } else if *getVersion { + fmt.Printf("%s %s\n", args[0], version) return nil, nil } arvCfg, err := loader.Load() if err != nil { - log.Fatal(err) + return nil, err } cfg := newConfig(logger, arvCfg) @@ -107,8 +101,7 @@ func main() { cfg, err := configure(logger, os.Args) if err != nil { - logger.Error(err) - os.Exit(1) + logger.Fatal(err) } else if cfg == nil { return } diff --git a/services/keepproxy/keepproxy.go b/services/keepproxy/keepproxy.go index dd67aff797..7c1360ad71 100644 --- a/services/keepproxy/keepproxy.go +++ b/services/keepproxy/keepproxy.go @@ -19,6 +19,7 @@ import ( "syscall" "time" + "git.arvados.org/arvados.git/lib/cmd" "git.arvados.org/arvados.git/lib/config" "git.arvados.org/arvados.git/sdk/go/arvados" "git.arvados.org/arvados.git/sdk/go/arvadosclient" @@ -42,19 +43,19 @@ var ( const rfc3339NanoFixed = "2006-01-02T15:04:05.000000000Z07:00" func configure(logger log.FieldLogger, args []string) (*arvados.Cluster, error) { - flags := flag.NewFlagSet(args[0], flag.ExitOnError) + prog := args[0] + flags := flag.NewFlagSet(prog, flag.ContinueOnError) dumpConfig := flags.Bool("dump-config", false, "write current configuration to stdout and exit") getVersion := flags.Bool("version", false, "Print version information and exit.") loader := config.NewLoader(os.Stdin, logger) loader.SetupFlags(flags) - args = loader.MungeLegacyConfigArgs(logger, args[1:], "-legacy-keepproxy-config") - flags.Parse(args) - // Print version information if requested - if *getVersion { + if ok, code := cmd.ParseFlags(flags, prog, args, "", os.Stderr); !ok { + os.Exit(code) + } else if *getVersion { fmt.Printf("keepproxy %s\n", version) return nil, nil } diff --git a/services/keepstore/command.go b/services/keepstore/command.go index 2a426936ed..94dabfda62 100644 --- a/services/keepstore/command.go +++ b/services/keepstore/command.go @@ -15,6 +15,7 @@ import ( "os" "sync" + "git.arvados.org/arvados.git/lib/cmd" "git.arvados.org/arvados.git/lib/config" "git.arvados.org/arvados.git/lib/service" "git.arvados.org/arvados.git/sdk/go/arvados" @@ -31,17 +32,18 @@ var ( ) func runCommand(prog string, args []string, stdin io.Reader, stdout, stderr io.Writer) int { - args, ok := convertKeepstoreFlagsToServiceFlags(args, ctxlog.FromContext(context.Background())) + args, ok, code := convertKeepstoreFlagsToServiceFlags(prog, args, ctxlog.FromContext(context.Background()), stderr) if !ok { - return 2 + return code } return Command.RunCommand(prog, args, stdin, stdout, stderr) } // Parse keepstore command line flags, and return equivalent -// service.Command flags. The second return value ("ok") is true if -// all provided flags were successfully converted. -func convertKeepstoreFlagsToServiceFlags(args []string, lgr logrus.FieldLogger) ([]string, bool) { +// service.Command flags. If the second return value ("ok") is false, +// the program should exit, and the third return value is a suitable +// exit code. +func convertKeepstoreFlagsToServiceFlags(prog string, args []string, lgr logrus.FieldLogger, stderr io.Writer) ([]string, bool, int) { flags := flag.NewFlagSet("", flag.ContinueOnError) flags.String("listen", "", "Services.Keepstore.InternalURLs") flags.Int("max-buffers", 0, "API.MaxKeepBlobBuffers") @@ -80,11 +82,8 @@ func convertKeepstoreFlagsToServiceFlags(args []string, lgr logrus.FieldLogger) flags.String("config", "", "") flags.String("legacy-keepstore-config", "", "") - err := flags.Parse(args) - if err == flag.ErrHelp { - return []string{"-help"}, true - } else if err != nil { - return nil, false + if ok, code := cmd.ParseFlags(flags, prog, args, "", stderr); !ok { + return nil, false, code } args = nil @@ -101,13 +100,13 @@ func convertKeepstoreFlagsToServiceFlags(args []string, lgr logrus.FieldLogger) } }) if !ok { - return nil, false + return nil, false, 2 } - flags = flag.NewFlagSet("", flag.ExitOnError) + flags = flag.NewFlagSet("", flag.ContinueOnError) loader := config.NewLoader(nil, lgr) loader.SetupFlags(flags) - return loader.MungeLegacyConfigArgs(lgr, args, "-legacy-keepstore-config"), true + return loader.MungeLegacyConfigArgs(lgr, args, "-legacy-keepstore-config"), true, 0 } type handler struct { diff --git a/tools/keep-block-check/keep-block-check.go b/tools/keep-block-check/keep-block-check.go index fec699f19f..995a1fd559 100644 --- a/tools/keep-block-check/keep-block-check.go +++ b/tools/keep-block-check/keep-block-check.go @@ -9,6 +9,7 @@ import ( "errors" "flag" "fmt" + "io" "io/ioutil" "log" "net/http" @@ -16,6 +17,7 @@ import ( "strings" "time" + "git.arvados.org/arvados.git/lib/cmd" "git.arvados.org/arvados.git/sdk/go/arvadosclient" "git.arvados.org/arvados.git/sdk/go/keepclient" ) @@ -23,13 +25,10 @@ import ( var version = "dev" func main() { - err := doMain(os.Args[1:]) - if err != nil { - log.Fatalf("%v", err) - } + os.Exit(doMain(os.Args[1:], os.Stderr)) } -func doMain(args []string) error { +func doMain(args []string, stderr io.Writer) int { flags := flag.NewFlagSet("keep-block-check", flag.ExitOnError) configFile := flags.String( @@ -69,33 +68,40 @@ func doMain(args []string) error { false, "Print version information and exit.") - // Parse args; omit the first arg which is the command name - flags.Parse(args) - - // Print version information if requested - if *getVersion { - fmt.Printf("keep-block-check %s\n", version) - os.Exit(0) + if ok, code := cmd.ParseFlags(flags, os.Args[0], args, "", stderr); !ok { + return code + } else if *getVersion { + fmt.Printf("%s %s\n", os.Args[0], version) + return 0 } config, blobSigningKey, err := loadConfig(*configFile) if err != nil { - return fmt.Errorf("Error loading configuration from file: %s", err.Error()) + fmt.Fprintf(stderr, "Error loading configuration from file: %s\n", err) + return 1 } // get list of block locators to be checked blockLocators, err := getBlockLocators(*locatorFile, *prefix) if err != nil { - return fmt.Errorf("Error reading block hashes to be checked from file: %s", err.Error()) + fmt.Fprintf(stderr, "Error reading block hashes to be checked from file: %s\n", err) + return 1 } // setup keepclient kc, blobSignatureTTL, err := setupKeepClient(config, *keepServicesJSON, *blobSignatureTTLFlag) if err != nil { - return fmt.Errorf("Error configuring keepclient: %s", err.Error()) + fmt.Fprintf(stderr, "Error configuring keepclient: %s\n", err) + return 1 + } + + err = performKeepBlockCheck(kc, blobSignatureTTL, blobSigningKey, blockLocators, *verbose) + if err != nil { + fmt.Fprintln(stderr, err) + return 1 } - return performKeepBlockCheck(kc, blobSignatureTTL, blobSigningKey, blockLocators, *verbose) + return 0 } type apiConfig struct { diff --git a/tools/keep-block-check/keep-block-check_test.go b/tools/keep-block-check/keep-block-check_test.go index e6519fb377..d973e06027 100644 --- a/tools/keep-block-check/keep-block-check_test.go +++ b/tools/keep-block-check/keep-block-check_test.go @@ -297,16 +297,18 @@ func (s *ServerRequiredSuite) TestLoadConfig(c *C) { func (s *DoMainTestSuite) Test_doMain_WithNoConfig(c *C) { args := []string{"-prefix", "a"} - err := doMain(args) - c.Check(err, NotNil) - c.Assert(strings.Contains(err.Error(), "config file not specified"), Equals, true) + var stderr bytes.Buffer + code := doMain(args, &stderr) + c.Check(code, Equals, 1) + c.Check(stderr.String(), Matches, ".*config file not specified\n") } func (s *DoMainTestSuite) Test_doMain_WithNoSuchConfigFile(c *C) { args := []string{"-config", "no-such-file"} - err := doMain(args) - c.Check(err, NotNil) - c.Assert(strings.Contains(err.Error(), "no such file or directory"), Equals, true) + var stderr bytes.Buffer + code := doMain(args, &stderr) + c.Check(code, Equals, 1) + c.Check(stderr.String(), Matches, ".*no such file or directory\n") } func (s *DoMainTestSuite) Test_doMain_WithNoBlockHashFile(c *C) { @@ -318,8 +320,10 @@ func (s *DoMainTestSuite) Test_doMain_WithNoBlockHashFile(c *C) { defer arvadostest.StopKeep(2) args := []string{"-config", config} - err := doMain(args) - c.Assert(strings.Contains(err.Error(), "block-hash-file not specified"), Equals, true) + var stderr bytes.Buffer + code := doMain(args, &stderr) + c.Check(code, Equals, 1) + c.Check(stderr.String(), Matches, ".*block-hash-file not specified\n") } func (s *DoMainTestSuite) Test_doMain_WithNoSuchBlockHashFile(c *C) { @@ -330,8 +334,10 @@ func (s *DoMainTestSuite) Test_doMain_WithNoSuchBlockHashFile(c *C) { defer arvadostest.StopKeep(2) args := []string{"-config", config, "-block-hash-file", "no-such-file"} - err := doMain(args) - c.Assert(strings.Contains(err.Error(), "no such file or directory"), Equals, true) + var stderr bytes.Buffer + code := doMain(args, &stderr) + c.Check(code, Equals, 1) + c.Check(stderr.String(), Matches, ".*no such file or directory\n") } func (s *DoMainTestSuite) Test_doMain(c *C) { @@ -346,9 +352,10 @@ func (s *DoMainTestSuite) Test_doMain(c *C) { defer os.Remove(locatorFile) args := []string{"-config", config, "-block-hash-file", locatorFile, "-v"} - err := doMain(args) - c.Check(err, NotNil) - c.Assert(err.Error(), Equals, "Block verification failed for 2 out of 2 blocks with matching prefix") + var stderr bytes.Buffer + code := doMain(args, &stderr) + c.Check(code, Equals, 1) + c.Assert(stderr.String(), Matches, "Block verification failed for 2 out of 2 blocks with matching prefix\n") checkErrorLog(c, []string{TestHash, TestHash2}, "Error verifying block", "Block not found") c.Assert(strings.Contains(logBuffer.String(), "Verifying block 1 of 2"), Equals, true) } diff --git a/tools/keep-exercise/keep-exercise.go b/tools/keep-exercise/keep-exercise.go index 84e1a6ce8e..1acd8d8b98 100644 --- a/tools/keep-exercise/keep-exercise.go +++ b/tools/keep-exercise/keep-exercise.go @@ -37,6 +37,7 @@ import ( "syscall" "time" + "git.arvados.org/arvados.git/lib/cmd" "git.arvados.org/arvados.git/lib/config" "git.arvados.org/arvados.git/sdk/go/arvados" "git.arvados.org/arvados.git/sdk/go/arvadosclient" @@ -89,12 +90,11 @@ func createKeepClient(lgr *log.Logger) (kc *keepclient.KeepClient) { } func main() { - flag.Parse() - - // Print version information if requested - if *getVersion { - fmt.Printf("keep-exercise %s\n", version) - os.Exit(0) + if ok, code := cmd.ParseFlags(flag.CommandLine, os.Args[0], os.Args[1:], "", os.Stderr); !ok { + os.Exit(code) + } else if *getVersion { + fmt.Printf("%s %s\n", os.Args[0], version) + return } lgr := log.New(os.Stderr, "", log.LstdFlags) diff --git a/tools/keep-rsync/keep-rsync.go b/tools/keep-rsync/keep-rsync.go index 6926a945e1..98c9609cb3 100644 --- a/tools/keep-rsync/keep-rsync.go +++ b/tools/keep-rsync/keep-rsync.go @@ -17,6 +17,7 @@ import ( "strings" "time" + "git.arvados.org/arvados.git/lib/cmd" "git.arvados.org/arvados.git/sdk/go/arvadosclient" "git.arvados.org/arvados.git/sdk/go/keepclient" ) @@ -76,12 +77,10 @@ func doMain() error { false, "Print version information and exit.") - // Parse args; omit the first arg which is the command name - flags.Parse(os.Args[1:]) - - // Print version information if requested - if *getVersion { - fmt.Printf("keep-rsync %s\n", version) + if ok, code := cmd.ParseFlags(flags, os.Args[0], os.Args[1:], "", os.Stderr); !ok { + os.Exit(code) + } else if *getVersion { + fmt.Printf("%s %s\n", os.Args[0], version) os.Exit(0) } diff --git a/tools/sync-groups/sync-groups.go b/tools/sync-groups/sync-groups.go index f0c3770783..e1e054a663 100644 --- a/tools/sync-groups/sync-groups.go +++ b/tools/sync-groups/sync-groups.go @@ -16,6 +16,7 @@ import ( "os" "strings" + "git.arvados.org/arvados.git/lib/cmd" "git.arvados.org/arvados.git/sdk/go/arvados" ) @@ -142,9 +143,9 @@ func ParseFlags(config *ConfigParams) error { * 1st: Group name * 2nd: User identifier * 3rd (Optional): User permission on the group: can_read, can_write or can_manage. (Default: can_write)` - fmt.Fprintf(os.Stderr, "%s\n\n", usageStr) - fmt.Fprintf(os.Stderr, "Usage:\n%s [OPTIONS] \n\n", os.Args[0]) - fmt.Fprintf(os.Stderr, "Options:\n") + fmt.Fprintf(flags.Output(), "%s\n\n", usageStr) + fmt.Fprintf(flags.Output(), "Usage:\n%s [OPTIONS] \n\n", os.Args[0]) + fmt.Fprintf(flags.Output(), "Options:\n") flags.PrintDefaults() } @@ -170,11 +171,9 @@ func ParseFlags(config *ConfigParams) error { "", "Use given group UUID as a parent for the remote groups. Should be owned by the system user. If not specified, a group named '"+config.ParentGroupName+"' will be used (and created if nonexistant).") - // Parse args; omit the first arg which is the command name - flags.Parse(os.Args[1:]) - - // Print version information if requested - if *getVersion { + if ok, code := cmd.ParseFlags(flags, os.Args[0], os.Args[1:], "input-file.csv", os.Stderr); !ok { + os.Exit(code) + } else if *getVersion { fmt.Printf("%s %s\n", os.Args[0], version) os.Exit(0) } -- 2.30.2