[ARVADOS] created: 2.1.0-1573-g47c3faf1e

Git user git at public.arvados.org
Thu Nov 11 20:44:22 UTC 2021


        at  47c3faf1e26be21190eeee7f266d44eb33a0aeb6 (commit)


commit 47c3faf1e26be21190eeee7f266d44eb33a0aeb6
Author: Tom Clegg <tom at curii.com>
Date:   Thu Nov 11 15:36:03 2021 -0500

    17840: Fix crash on config load error.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/services/crunch-dispatch-local/crunch-dispatch-local.go b/services/crunch-dispatch-local/crunch-dispatch-local.go
index be4225276..c9cbdd01f 100644
--- a/services/crunch-dispatch-local/crunch-dispatch-local.go
+++ b/services/crunch-dispatch-local/crunch-dispatch-local.go
@@ -72,6 +72,10 @@ func main() {
 
 	loader := config.NewLoader(nil, logger)
 	cfg, err := loader.Load()
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "error loading config: %s\n", err)
+		os.Exit(1)
+	}
 	cluster, err := cfg.GetCluster("")
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "config error: %s\n", err)

commit 37d9f94b06ff367a3514b58ec6f0e4d4d0116030
Author: Tom Clegg <tom at curii.com>
Date:   Thu Nov 11 13:50:36 2021 -0500

    17840: Deduplicate flag-parsing code.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/cmd/arvados-client/container_gateway.go b/cmd/arvados-client/container_gateway.go
index 5359e00c6..aca6c5b79 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 b6bbb9799..96241d24b 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 3c4b560c9..0ec79e117 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 000000000..3e872fcd1
--- /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 c852b0b54..eeab6ac8c 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 241f37683..a5cc28b80 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 6065ad2c0..f2a7af493 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 4a48db1a8..a3673c979 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 2975e3b3d..b78b288ab 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 a5e69387e..f9bf3df9d 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 bb3405a49..2f9521c65 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 ...] <collection-uuid> <collection-uuid> ...
 
-  %s [options ...] <collection-pdh>,<collection_uuid> \
-     <collection-pdh>,<collection_uuid> ...
+  %s [options ...] <collection-pdh>,<collection-uuid> \
+     <collection-pdh>,<collection-uuid> ...
 
   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 8663c6ee5..71fe1c5dc 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 ff00ee1e3..ca4b68147 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 8b2fbfdd0..1d063506b 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 d6d1ae030..f88d977c4 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 980b7d2ae..44eb61e7f 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 da466c31c..5038e4788 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 268aba751..880799b34 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 af63b32ab..b926ac273 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 159ee69b1..be4225276 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 02493bf4f..ff1077fae 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 b2d157cbd..68a2305f7 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 3ff3912e8..6383eae54 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 49cb17dfa..8a95d389c 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 4c6616522..aa97a18ad 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 dd67aff79..7c1360ad7 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 2a426936e..94dabfda6 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 fec699f19..995a1fd55 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 e6519fb37..d973e0602 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 84e1a6ce8..1acd8d8b9 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 6926a945e..98c9609cb 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 f0c377078..e1e054a66 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] <input-file.csv>\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] <input-file.csv>\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)
 	}

commit 00900388c9704a4fe76a459934a2b9f73a3cec1a
Author: Tom Clegg <tom at curii.com>
Date:   Wed Nov 10 15:10:06 2021 -0500

    17840: Fix duplicate error message re parsing command line args.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/services/keep-balance/main.go b/services/keep-balance/main.go
index 37ed06369..49cb17dfa 100644
--- a/services/keep-balance/main.go
+++ b/services/keep-balance/main.go
@@ -62,10 +62,9 @@ func runCommand(prog string, args []string, stdin io.Reader, stdout, stderr io.W
 	if err == flag.ErrHelp {
 		return 0
 	} else if err != nil {
-		logger.Errorf("error parsing command line flags: %s", err)
 		return 2
 	} else if flags.NArg() != 0 {
-		logger.Errorf("unrecognized command line arguments: %v", flags.Args())
+		fmt.Fprintf(stderr, "unrecognized command line arguments: %v", flags.Args())
 		return 2
 	}
 

commit 40f551004ab4e5f1d8ab02ddb55dca225ee8f6ac
Author: Tom Clegg <tom at curii.com>
Date:   Mon Nov 8 14:22:20 2021 -0500

    17840: Check for unparsed command line arguments.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/cmd/arvados-package/cmd.go b/cmd/arvados-package/cmd.go
index 54f0809d6..863bbe925 100644
--- a/cmd/arvados-package/cmd.go
+++ b/cmd/arvados-package/cmd.go
@@ -124,7 +124,7 @@ Options:
 	if err != nil {
 		return opts, err
 	}
-	if len(flags.Args()) > 0 {
+	if flags.NArg() != 0 {
 		return opts, fmt.Errorf("unrecognized command line arguments: %v", flags.Args())
 	}
 	if opts.SourceDir == "" {
diff --git a/lib/boot/cmd.go b/lib/boot/cmd.go
index 001504e20..b6bbb9799 100644
--- a/lib/boot/cmd.go
+++ b/lib/boot/cmd.go
@@ -75,6 +75,8 @@ func (bcmd bootCommand) run(ctx context.Context, prog string, args []string, std
 		return nil
 	} else if err != nil {
 		return err
+	} else if flags.NArg() != 0 {
+		return fmt.Errorf("unrecognized command line arguments: %v", flags.Args())
 	} 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 4816f20ee..3c4b560c9 100644
--- a/lib/cloud/cloudtest/cmd.go
+++ b/lib/cloud/cloudtest/cmd.go
@@ -47,10 +47,8 @@ func (command) RunCommand(prog string, args []string, stdin io.Reader, stdout, s
 		return 0
 	} else if err != nil {
 		return 2
-	}
-
-	if len(flags.Args()) != 0 {
-		flags.Usage()
+	} else if flags.NArg() != 0 {
+		err = fmt.Errorf("unrecognized command line arguments: %v", flags.Args())
 		return 2
 	}
 	logger := ctxlog.New(stderr, "text", "info")
diff --git a/lib/config/cmd.go b/lib/config/cmd.go
index 8e638e6ec..c852b0b54 100644
--- a/lib/config/cmd.go
+++ b/lib/config/cmd.go
@@ -44,10 +44,8 @@ func (dumpCommand) RunCommand(prog string, args []string, stdin io.Reader, stdou
 		return 0
 	} else if err != nil {
 		return 2
-	}
-
-	if len(flags.Args()) != 0 {
-		flags.Usage()
+	} else if flags.NArg() != 0 {
+		err = fmt.Errorf("unrecognized command line arguments: %v", flags.Args())
 		return 2
 	}
 
@@ -98,10 +96,8 @@ func (checkCommand) RunCommand(prog string, args []string, stdin io.Reader, stdo
 		return 0
 	} else if err != nil {
 		return 2
-	}
-
-	if len(flags.Args()) != 0 {
-		flags.Usage()
+	} else if flags.NArg() != 0 {
+		err = fmt.Errorf("unrecognized command line arguments: %v", flags.Args())
 		return 2
 	}
 
diff --git a/lib/config/deprecated_test.go b/lib/config/deprecated_test.go
index 595e4c9ca..4206ef577 100644
--- a/lib/config/deprecated_test.go
+++ b/lib/config/deprecated_test.go
@@ -35,7 +35,9 @@ func testLoadLegacyConfig(content []byte, mungeFlag string, c *check.C) (*arvado
 	ldr := testLoader(c, "Clusters: {zzzzz: {}}", nil)
 	ldr.SetupFlags(flags)
 	args := ldr.MungeLegacyConfigArgs(ldr.Logger, []string{"-config", tmpfile.Name()}, mungeFlag)
-	flags.Parse(args)
+	err = flags.Parse(args)
+	c.Assert(err, check.IsNil)
+	c.Assert(flags.NArg(), check.Equals, 0)
 	cfg, err := ldr.Load()
 	if err != nil {
 		return nil, err
diff --git a/lib/crunchrun/crunchrun.go b/lib/crunchrun/crunchrun.go
index 8f3a30203..a5e69387e 100644
--- a/lib/crunchrun/crunchrun.go
+++ b/lib/crunchrun/crunchrun.go
@@ -1699,6 +1699,10 @@ func (command) RunCommand(prog string, args []string, stdin io.Reader, stdout, s
 	} else if err != nil {
 		log.Print(err)
 		return 1
+	} else if flags.NArg() != 1 {
+		fmt.Fprintf(flags.Output(), "Usage: %s [options] containerUUID\n\nOptions:\n", prog)
+		flags.PrintDefaults()
+		return 2
 	}
 
 	containerUUID := flags.Arg(0)
diff --git a/lib/diagnostics/cmd.go b/lib/diagnostics/cmd.go
index b0241b3ae..8663c6ee5 100644
--- a/lib/diagnostics/cmd.go
+++ b/lib/diagnostics/cmd.go
@@ -39,6 +39,9 @@ func (cmd Command) RunCommand(prog string, args []string, stdin io.Reader, stdou
 	} 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
 	}
 	diag.logger = ctxlog.New(stdout, "text", diag.logLevel)
 	diag.logger.SetFormatter(&logrus.TextFormatter{DisableTimestamp: true, DisableLevelTruncation: true, PadLevelText: true})
diff --git a/lib/install/init.go b/lib/install/init.go
index 7ae42c531..8b2fbfdd0 100644
--- a/lib/install/init.go
+++ b/lib/install/init.go
@@ -67,7 +67,7 @@ func (initcmd *initCommand) RunCommand(prog string, args []string, stdin io.Read
 		return 2
 	} else if *versionFlag {
 		return cmd.Version.RunCommand(prog, args, stdin, stdout, stderr)
-	} else if len(flags.Args()) > 0 {
+	} 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) {
diff --git a/lib/mount/command.go b/lib/mount/command.go
index e92af2407..d6d1ae030 100644
--- a/lib/mount/command.go
+++ b/lib/mount/command.go
@@ -9,6 +9,7 @@ import (
 	"io"
 	"log"
 	"net/http"
+
 	// pprof is only imported to register its HTTP handlers
 	_ "net/http/pprof"
 	"os"
diff --git a/lib/service/cmd.go b/lib/service/cmd.go
index e67c24f65..268aba751 100644
--- a/lib/service/cmd.go
+++ b/lib/service/cmd.go
@@ -78,6 +78,9 @@ func (c *command) RunCommand(prog string, args []string, stdin io.Reader, stdout
 		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
 	} 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 4e5596461..af63b32ab 100644
--- a/services/arv-git-httpd/main.go
+++ b/services/arv-git-httpd/main.go
@@ -23,7 +23,7 @@ func main() {
 		TimestampFormat: "2006-01-02T15:04:05.000000000Z07:00",
 	})
 
-	flags := flag.NewFlagSet(os.Args[0], flag.ExitOnError)
+	flags := flag.NewFlagSet(os.Args[0], flag.ContinueOnError)
 	loader := config.NewLoader(os.Stdin, logger)
 	loader.SetupFlags(flags)
 
@@ -31,7 +31,16 @@ func main() {
 	getVersion := flags.Bool("version", false, "print version information and exit.")
 
 	args := loader.MungeLegacyConfigArgs(logger, os.Args[1:], "-legacy-git-httpd-config")
-	flags.Parse(args)
+	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 {
 		fmt.Printf("arv-git-httpd %s\n", version)
diff --git a/services/crunch-dispatch-local/crunch-dispatch-local.go b/services/crunch-dispatch-local/crunch-dispatch-local.go
index a3cb1341a..159ee69b1 100644
--- a/services/crunch-dispatch-local/crunch-dispatch-local.go
+++ b/services/crunch-dispatch-local/crunch-dispatch-local.go
@@ -67,7 +67,14 @@ func doMain() error {
 		"Print version information and exit.")
 
 	// Parse args; omit the first arg which is the command name
-	flags.Parse(os.Args[1:])
+	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())
+	}
 
 	// Print version information if requested
 	if *getVersion {
diff --git a/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go b/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go
index 584db38ed..02493bf4f 100644
--- a/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go
+++ b/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go
@@ -91,13 +91,14 @@ func (disp *Dispatcher) configure(prog string, args []string) error {
 		false,
 		"Print version information and exit.")
 
-	args = loader.MungeLegacyConfigArgs(logrus.StandardLogger(), args, "-legacy-crunch-dispatch-slurm-config")
-
-	// Parse args; omit the first arg which is the command name
+	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())
 	}
 
 	// Print version information if requested
diff --git a/services/crunchstat/crunchstat.go b/services/crunchstat/crunchstat.go
index 6bf2e3399..3ff3912e8 100644
--- a/services/crunchstat/crunchstat.go
+++ b/services/crunchstat/crunchstat.go
@@ -32,15 +32,22 @@ func main() {
 		Logger: log.New(os.Stderr, "crunchstat: ", 0),
 	}
 
-	flag.StringVar(&reporter.CgroupRoot, "cgroup-root", "", "Root of cgroup tree")
-	flag.StringVar(&reporter.CgroupParent, "cgroup-parent", "", "Name of container parent under cgroup")
-	flag.StringVar(&reporter.CIDFile, "cgroup-cid", "", "Path to container id file")
-	flag.IntVar(&signalOnDeadPPID, "signal-on-dead-ppid", signalOnDeadPPID, "Signal to send child if crunchstat's parent process disappears (0 to disable)")
-	flag.DurationVar(&ppidCheckInterval, "ppid-check-interval", ppidCheckInterval, "Time between checks for parent process disappearance")
-	pollMsec := flag.Int64("poll", 1000, "Reporting interval, in milliseconds")
-	getVersion := flag.Bool("version", false, "Print version information and exit.")
-
-	flag.Parse()
+	flags := flag.NewFlagSet(os.Args[0], flag.ExitOnError)
+	flags.StringVar(&reporter.CgroupRoot, "cgroup-root", "", "Root of cgroup tree")
+	flags.StringVar(&reporter.CgroupParent, "cgroup-parent", "", "Name of container parent under cgroup")
+	flags.StringVar(&reporter.CIDFile, "cgroup-cid", "", "Path to container id file")
+	flags.IntVar(&signalOnDeadPPID, "signal-on-dead-ppid", signalOnDeadPPID, "Signal to send child if crunchstat's parent process disappears (0 to disable)")
+	flags.DurationVar(&ppidCheckInterval, "ppid-check-interval", ppidCheckInterval, "Time between checks for parent process disappearance")
+	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 {
@@ -48,6 +55,12 @@ func main() {
 		return
 	}
 
+	if flags.NArg() == 0 {
+		fmt.Fprintf(flags.Output(), "Usage: %s [options] program [args...]\n\nOptions:\n", os.Args[0])
+		flags.PrintDefaults()
+		os.Exit(2)
+	}
+
 	reporter.Logger.Printf("crunchstat %s started", version)
 
 	if reporter.CgroupRoot == "" {
@@ -58,7 +71,7 @@ func main() {
 	reporter.PollPeriod = time.Duration(*pollMsec) * time.Millisecond
 
 	reporter.Start()
-	err := runCommand(flag.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 605ee5fc8..37ed06369 100644
--- a/services/keep-balance/main.go
+++ b/services/keep-balance/main.go
@@ -32,7 +32,7 @@ func runCommand(prog string, args []string, stdin io.Reader, stdout, stderr io.W
 	logger := ctxlog.FromContext(context.Background())
 
 	var options RunOptions
-	flags := flag.NewFlagSet(prog, flag.ExitOnError)
+	flags := flag.NewFlagSet(prog, flag.ContinueOnError)
 	flags.BoolVar(&options.Once, "once", false,
 		"balance once and then exit")
 	flags.BoolVar(&options.CommitPulls, "commit-pulls", false,
@@ -41,9 +41,12 @@ func runCommand(prog string, args []string, stdin io.Reader, stdout, stderr io.W
 		"send trash requests (delete unreferenced old blocks, and excess replicas of overreplicated blocks)")
 	flags.BoolVar(&options.CommitConfirmedFields, "commit-confirmed-fields", true,
 		"update collection fields (replicas_confirmed, storage_classes_confirmed, etc.)")
-	flags.Bool("version", false, "Write version information to stdout and exit 0")
 	dumpFlag := flags.Bool("dump", false, "dump details for each block to stdout")
 	pprofAddr := flags.String("pprof", "", "serve Go profile data at `[addr]:port`")
+	// "show version" is implemented by service.Command, so we
+	// don't need the var here -- we just need the -version flag
+	// to pass flags.Parse().
+	flags.Bool("version", false, "Write version information to stdout and exit 0")
 
 	if *pprofAddr != "" {
 		go func() {
@@ -56,12 +59,13 @@ func runCommand(prog string, args []string, stdin io.Reader, stdout, stderr io.W
 
 	munged := loader.MungeLegacyConfigArgs(logger, args, "-legacy-keepbalance-config")
 	err := flags.Parse(munged)
-	if err != nil {
+	if err == flag.ErrHelp {
+		return 0
+	} else if err != nil {
 		logger.Errorf("error parsing command line flags: %s", err)
 		return 2
-	}
-	if flags.NArg() != 0 {
-		logger.Errorf("error parsing command line flags: extra arguments: %q", flags.Args())
+	} else if flags.NArg() != 0 {
+		logger.Errorf("unrecognized command line arguments: %v", flags.Args())
 		return 2
 	}
 
@@ -84,7 +88,7 @@ func runCommand(prog string, args []string, stdin io.Reader, stdout, stderr io.W
 	}
 	flags.Visit(func(f *flag.Flag) {
 		if !dropFlag[f.Name] {
-			args = append(args, "-"+f.Name, f.Value.String())
+			args = append(args, "-"+f.Name+"="+f.Value.String())
 		}
 	})
 
diff --git a/services/keep-balance/main_test.go b/services/keep-balance/main_test.go
index 65a2d5567..820f35216 100644
--- a/services/keep-balance/main_test.go
+++ b/services/keep-balance/main_test.go
@@ -28,6 +28,7 @@ func (s *mainSuite) TestVersionFlag(c *check.C) {
 	runCommand("keep-balance", []string{"-version"}, nil, &stdout, &stderr)
 	c.Check(stderr.String(), check.Equals, "")
 	c.Log(stdout.String())
+	c.Check(stdout.String(), check.Matches, `keep-balance.*\(go1.*\)\n`)
 }
 
 func (s *mainSuite) TestHTTPServer(c *check.C) {
diff --git a/services/keep-web/main.go b/services/keep-web/main.go
index a9ac834a2..4c6616522 100644
--- a/services/keep-web/main.go
+++ b/services/keep-web/main.go
@@ -58,8 +58,8 @@ func init() {
 	})
 }
 
-func configure(logger log.FieldLogger, args []string) *Config {
-	flags := flag.NewFlagSet(args[0], flag.ExitOnError)
+func configure(logger log.FieldLogger, args []string) (*Config, error) {
+	flags := flag.NewFlagSet(args[0], flag.ContinueOnError)
 
 	loader := config.NewLoader(os.Stdin, logger)
 	loader.SetupFlags(flags)
@@ -70,12 +70,19 @@ func configure(logger log.FieldLogger, args []string) *Config {
 		"print version information and exit.")
 
 	args = loader.MungeLegacyConfigArgs(logger, args[1:], "-legacy-keepweb-config")
-	flags.Parse(args)
+	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)
-		return nil
+		return nil, nil
 	}
 
 	arvCfg, err := loader.Load()
@@ -87,22 +94,22 @@ func configure(logger log.FieldLogger, args []string) *Config {
 	if *dumpConfig {
 		out, err := yaml.Marshal(cfg)
 		if err != nil {
-			log.Fatal(err)
+			return nil, err
 		}
 		_, err = os.Stdout.Write(out)
-		if err != nil {
-			log.Fatal(err)
-		}
-		return nil
+		return nil, err
 	}
-	return cfg
+	return cfg, nil
 }
 
 func main() {
 	logger := log.New()
 
-	cfg := configure(logger, os.Args)
-	if cfg == nil {
+	cfg, err := configure(logger, os.Args)
+	if err != nil {
+		logger.Error(err)
+		os.Exit(1)
+	} else if cfg == nil {
 		return
 	}
 

commit 54781f3d3c1dea0e14542d129b1c8e061ad406fc
Author: Tom Clegg <tom at curii.com>
Date:   Tue Jun 29 10:11:40 2021 -0400

    17840: Check for unparsed command line arguments.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/services/keep-balance/main.go b/services/keep-balance/main.go
index e1573e7f7..605ee5fc8 100644
--- a/services/keep-balance/main.go
+++ b/services/keep-balance/main.go
@@ -55,7 +55,15 @@ func runCommand(prog string, args []string, stdin io.Reader, stdout, stderr io.W
 	loader.SetupFlags(flags)
 
 	munged := loader.MungeLegacyConfigArgs(logger, args, "-legacy-keepbalance-config")
-	flags.Parse(munged)
+	err := flags.Parse(munged)
+	if err != nil {
+		logger.Errorf("error parsing command line flags: %s", err)
+		return 2
+	}
+	if flags.NArg() != 0 {
+		logger.Errorf("error parsing command line flags: extra arguments: %q", flags.Args())
+		return 2
+	}
 
 	if *dumpFlag {
 		dumper := logrus.New()

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list