[lxc-devel] [lxd/master] Normalize sub commands invocation

freeekanayaka on Github lxc-bot at linuxcontainers.org
Tue Aug 29 09:09:40 UTC 2017


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 2523 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20170829/b474f48b/attachment.bin>
-------------- next part --------------
From a5bd6ee51f6014b6eaf11380fb500b19d9ada5ad Mon Sep 17 00:00:00 2001
From: Free Ekanayaka <free.ekanayaka at canonical.com>
Date: Sun, 20 Aug 2017 17:46:39 +0000
Subject: [PATCH 1/4] Add a cmd.Parser helper for parsing command line flags

This helper parses a command line according to the attributes defined
on a given object, which will be populated with the parsed values.

It will be used to replace the global command line variables
defined in main.go, making it easier to create unit tests that
exercise whole commands.

Signed-off-by: Free Ekanayaka <free.ekanayaka at canonical.com>
---
 shared/cmd/parser.go      | 142 ++++++++++++++++++++++++++++++++++++++++
 shared/cmd/parser_test.go | 162 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 304 insertions(+)
 create mode 100644 shared/cmd/parser.go
 create mode 100644 shared/cmd/parser_test.go

diff --git a/shared/cmd/parser.go b/shared/cmd/parser.go
new file mode 100644
index 000000000..ce71eaeeb
--- /dev/null
+++ b/shared/cmd/parser.go
@@ -0,0 +1,142 @@
+package cmd
+
+import (
+	"reflect"
+	"strings"
+	"unsafe"
+
+	"github.com/lxc/lxd/shared/gnuflag"
+)
+
+// Parser for command line arguments.
+type Parser struct {
+	Context      *Context
+	UsageMessage string
+	ExitOnError  bool
+}
+
+// NewParser returns a Parser connected to the given I/O context and printing
+// the given usage message when '--help' or '-h' are passed.
+func NewParser(context *Context, usage string) *Parser {
+	return &Parser{
+		Context:      context,
+		UsageMessage: usage,
+		ExitOnError:  true,
+	}
+}
+
+// Parse a command line populating the given args object accordingly.
+//
+// The command line format is expected to be:
+//
+// <cmd> [subcmd [params]] [flags] [-- [extra]]
+//
+// The args object may have Subcommand, Params and Extra attributes
+// (respectively of type string, []string and []string), which will be
+// populated with the subcommand, its params and any extra argument (if
+// present).
+//
+// The type of the args object must have one attribute for each supported
+// command line flag, annotated with a tag like `flag:"<name>"`, where <name>
+// is the name of the command line flag.
+//
+// In case of parsing error (e.g. unknown command line flag) the default
+// behavior is to call os.Exit() with a non-zero value. This can be disabled by
+// setting the ExitOnError attribute to false, in which case the error will be
+// returned.
+func (p *Parser) Parse(line []string, args interface{}) error {
+	val := reflect.ValueOf(args).Elem()
+
+	if err := p.parseFlags(line, val); err != nil {
+		return err
+	}
+
+	p.parseRest(line, val)
+
+	return nil
+}
+
+// Populate the given FlagSet by introspecting the given object, adding a new
+// flag variable for each annotated attribute.
+func (p *Parser) parseFlags(line []string, val reflect.Value) error {
+	mode := gnuflag.ContinueOnError
+	if p.ExitOnError {
+		mode = gnuflag.ExitOnError
+	}
+
+	flags := gnuflag.NewFlagSet(line[0], mode)
+	flags.SetOutput(p.Context.stderr)
+
+	if p.UsageMessage != "" {
+		// Since usage will be printed only if "-h" or "--help" are
+		// explicitly set in the command line, use stdout for it.
+		flags.Usage = func() {
+			p.Context.Output(p.UsageMessage)
+		}
+	}
+
+	typ := val.Type()
+	for i := 0; i < typ.NumField(); i++ {
+		name := typ.Field(i).Tag.Get("flag")
+		if name == "" {
+			continue
+		}
+		kind := typ.Field(i).Type.Kind()
+		addr := val.Field(i).Addr()
+		switch kind {
+		case reflect.Bool:
+			pointer := (*bool)(unsafe.Pointer(addr.Pointer()))
+			flags.BoolVar(pointer, name, false, "")
+		case reflect.String:
+			pointer := (*string)(unsafe.Pointer(addr.Pointer()))
+			flags.StringVar(pointer, name, "", "")
+		case reflect.Int:
+			pointer := (*int)(unsafe.Pointer(addr.Pointer()))
+			flags.IntVar(pointer, name, -1, "")
+		case reflect.Int64:
+			pointer := (*int64)(unsafe.Pointer(addr.Pointer()))
+			flags.Int64Var(pointer, name, -1, "")
+		}
+	}
+
+	return flags.Parse(true, line[1:])
+}
+
+// Parse any non-flag argument, i.e. the subcommand, its parameters and any
+// extra argument following "--".
+func (p *Parser) parseRest(line []string, val reflect.Value) {
+	subcommand := ""
+	params := []string{}
+	extra := []string{}
+	if len(line) > 1 {
+		rest := line[1:]
+		for i, token := range rest {
+			if token == "--" {
+				// Set extra to anything left, excluding the token.
+				if i < len(rest)-1 {
+					extra = rest[i+1:]
+				}
+				break
+			}
+			if strings.HasPrefix(token, "-") {
+				// Subcommand and parameters must both come
+				// before any flag.
+				break
+			}
+			if i == 0 {
+				subcommand = token
+				continue
+			}
+			params = append(params, token)
+		}
+	}
+	if field := val.FieldByName("Subcommand"); field.IsValid() {
+		field.SetString(subcommand)
+	}
+	if field := val.FieldByName("Params"); field.IsValid() {
+		field.Set(reflect.ValueOf(params))
+	}
+	if field := val.FieldByName("Extra"); field.IsValid() {
+		field.Set(reflect.ValueOf(extra))
+	}
+}
diff --git a/shared/cmd/parser_test.go b/shared/cmd/parser_test.go
new file mode 100644
index 000000000..1111a168e
--- /dev/null
+++ b/shared/cmd/parser_test.go
@@ -0,0 +1,162 @@
+package cmd_test
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/mpvl/subtest"
+	"github.com/stretchr/testify/assert"
+
+	"github.com/lxc/lxd/shared/cmd"
+)
+
+// Sample command line arguments specification.
+type Args struct {
+	Subcommand string
+	Params     []string
+	Extra      []string
+
+	Help      bool   `flag:"help"`
+	Text      string `flag:"text"`
+	Number    int    `flag:"number"`
+	BigNumber int64  `flag:"big-number"`
+}
+
+// Check the default values of all command line args.
+func TestParser_ArgsDefaults(t *testing.T) {
+	line := []string{"cmd"}
+	args := &Args{}
+	parser := newParser()
+
+	assert.NoError(t, parser.Parse(line, args))
+
+	assert.Equal(t, "", args.Text)
+	assert.Equal(t, false, args.Help)
+	assert.Equal(t, -1, args.Number)
+	assert.Equal(t, int64(-1), args.BigNumber)
+}
+
+// Check that parsing the command line results in the correct attributes
+// being set.
+func TestParser_ArgsCustom(t *testing.T) {
+	line := []string{
+		"cmd",
+		"--text", "hello",
+		"--help",
+		"--number", "10",
+		"--big-number", "666",
+	}
+	args := &Args{}
+	parser := newParser()
+
+	assert.NoError(t, parser.Parse(line, args))
+
+	assert.Equal(t, "hello", args.Text)
+	assert.Equal(t, true, args.Help)
+	assert.Equal(t, 10, args.Number)
+	assert.Equal(t, int64(666), args.BigNumber)
+}
+
+// Check that the subcommand is properly set.
+func TestParser_Subcommand(t *testing.T) {
+	cases := []struct {
+		line       []string
+		subcommand string
+	}{
+		{[]string{"cmd"}, ""},
+		{[]string{"cmd", "--help"}, ""},
+		{[]string{"cmd", "subcmd"}, "subcmd"},
+		{[]string{"cmd", "subcmd", "--help"}, "subcmd"},
+		{[]string{"cmd", "--help", "subcmd"}, ""},
+	}
+	for _, c := range cases {
+		subtest.Run(t, strings.Join(c.line, "_"), func(t *testing.T) {
+			args := &Args{}
+			parser := newParser()
+			assert.NoError(t, parser.Parse(c.line, args))
+			assert.Equal(t, c.subcommand, args.Subcommand)
+		})
+	}
+}
+
+// Check that subcommand params are properly set.
+func TestParser_Params(t *testing.T) {
+	cases := []struct {
+		line   []string
+		params []string
+	}{
+		{[]string{"cmd"}, []string{}},
+		{[]string{"cmd", "--help"}, []string{}},
+		{[]string{"cmd", "subcmd"}, []string{}},
+		{[]string{"cmd", "subcmd", "param"}, []string{"param"}},
+		{[]string{"cmd", "subcmd", "param1", "param2"}, []string{"param1", "param2"}},
+		{[]string{"cmd", "subcmd", "param", "--help"}, []string{"param"}},
+		{[]string{"cmd", "subcmd", "--help", "param"}, []string{}},
+	}
+	for _, c := range cases {
+		subtest.Run(t, strings.Join(c.line, "_"), func(t *testing.T) {
+			args := &Args{}
+			parser := newParser()
+			assert.NoError(t, parser.Parse(c.line, args))
+			assert.Equal(t, c.params, args.Params)
+		})
+	}
+}
+
+// Check that extra params are properly set.
+func TestParser_Extra(t *testing.T) {
+	cases := []struct {
+		line  []string
+		extra []string
+	}{
+		{[]string{"cmd"}, []string{}},
+		{[]string{"cmd", "--help"}, []string{}},
+		{[]string{"cmd", "subcmd"}, []string{}},
+		{[]string{"cmd", "subcmd", "--"}, []string{}},
+		{[]string{"cmd", "subcmd", "--", "extra"}, []string{"extra"}},
+		{[]string{"cmd", "subcmd", "--", "extra1", "--extra2"}, []string{"extra1", "--extra2"}},
+	}
+	for _, c := range cases {
+		subtest.Run(t, strings.Join(c.line, "_"), func(t *testing.T) {
+			args := &Args{}
+			parser := newParser()
+			assert.NoError(t, parser.Parse(c.line, args))
+			assert.Equal(t, c.extra, args.Extra)
+		})
+	}
+}
+
+// If a flag doesn't exist, an error is returned.
+func TestParser_Error(t *testing.T) {
+	line := []string{"cmd", "--boom"}
+	args := &Args{}
+	parser := newParser()
+
+	assert.Error(t, parser.Parse(line, args))
+}
+
+// If a usage string is passed, and the command line has the help flag, the
+// message is printed out.
+func TestParser_Usage(t *testing.T) {
+	line := []string{"cmd", "-h"}
+	args := &Args{}
+	streams := cmd.NewMemoryStreams("")
+
+	parser := newParserWithStreams(streams)
+	parser.UsageMessage = "usage message"
+
+	assert.Error(t, parser.Parse(line, args))
+	assert.Equal(t, parser.UsageMessage, streams.Out())
+}
+
+// Return a new test parser
+func newParser() *cmd.Parser {
+	return newParserWithStreams(cmd.NewMemoryStreams(""))
+}
+
+// Return a new test parser using the given streams for its context.
+func newParserWithStreams(streams *cmd.MemoryStreams) *cmd.Parser {
+	return &cmd.Parser{
+		Context: cmd.NewMemoryContext(streams),
+	}
+}

From 04a37e8141475da71f3a55b8fb5fb3cff980da2e Mon Sep 17 00:00:00 2001
From: Free Ekanayaka <free.ekanayaka at canonical.com>
Date: Sun, 20 Aug 2017 18:17:03 +0000
Subject: [PATCH 2/4] Plug cmd.Parser into main.go

Replace global command line variables in main.go with a new Args
structure, populated using cmd.Parser.

Signed-off-by: Free Ekanayaka <free.ekanayaka at canonical.com>
---
 lxd/main.go                    | 186 +++++++----------------------------------
 lxd/main_args.go               | 134 +++++++++++++++++++++++++++++
 lxd/main_args_test.go          |  95 +++++++++++++++++++++
 lxd/main_callhook.go           |  10 +--
 lxd/main_daemon.go             |  16 ++--
 lxd/main_forkexec.go           |  17 ++--
 lxd/main_forkmigrate.go        |  16 ++--
 lxd/main_forkstart.go          |  12 +--
 lxd/main_import.go             |   9 +-
 lxd/main_init.go               |  29 +------
 lxd/main_init_test.go          |   5 +-
 lxd/main_migratedumpsuccess.go |  10 +--
 lxd/main_netcat.go             |  18 ++--
 lxd/main_shutdown.go           |   8 +-
 lxd/main_waitready.go          |   6 +-
 shared/cmd/context.go          |   7 ++
 16 files changed, 336 insertions(+), 242 deletions(-)
 create mode 100644 lxd/main_args.go
 create mode 100644 lxd/main_args_test.go

diff --git a/lxd/main.go b/lxd/main.go
index b8b92c136..ffdc279a0 100644
--- a/lxd/main.go
+++ b/lxd/main.go
@@ -7,35 +7,12 @@ import (
 	"time"
 
 	"github.com/lxc/lxd/shared"
-	"github.com/lxc/lxd/shared/gnuflag"
+	"github.com/lxc/lxd/shared/cmd"
 	"github.com/lxc/lxd/shared/logger"
 	"github.com/lxc/lxd/shared/logging"
 	"github.com/lxc/lxd/shared/version"
 )
 
-// Global arguments
-var argAuto = gnuflag.Bool("auto", false, "")
-var argPreseed = gnuflag.Bool("preseed", false, "")
-var argCPUProfile = gnuflag.String("cpuprofile", "", "")
-var argDebug = gnuflag.Bool("debug", false, "")
-var argGroup = gnuflag.String("group", "", "")
-var argHelp = gnuflag.Bool("help", false, "")
-var argLogfile = gnuflag.String("logfile", "", "")
-var argMemProfile = gnuflag.String("memprofile", "", "")
-var argNetworkAddress = gnuflag.String("network-address", "", "")
-var argNetworkPort = gnuflag.Int64("network-port", -1, "")
-var argPrintGoroutinesEvery = gnuflag.Int("print-goroutines-every", -1, "")
-var argStorageBackend = gnuflag.String("storage-backend", "", "")
-var argStorageCreateDevice = gnuflag.String("storage-create-device", "", "")
-var argStorageCreateLoop = gnuflag.Int64("storage-create-loop", -1, "")
-var argStorageDataset = gnuflag.String("storage-pool", "", "")
-var argSyslog = gnuflag.Bool("syslog", false, "")
-var argTimeout = gnuflag.Int("timeout", -1, "")
-var argTrustPassword = gnuflag.String("trust-password", "", "")
-var argVerbose = gnuflag.Bool("verbose", false, "")
-var argVersion = gnuflag.Bool("version", false, "")
-var argForce = gnuflag.Bool("force", false, "")
-
 // Global variables
 var debug bool
 var verbose bool
@@ -58,122 +35,24 @@ func main() {
 }
 
 func run() error {
-	// Our massive custom usage
-	gnuflag.Usage = func() {
-		fmt.Printf("Usage: lxd [command] [options]\n")
-
-		fmt.Printf("\nCommands:\n")
-		fmt.Printf("    activateifneeded\n")
-		fmt.Printf("        Check if LXD should be started (at boot) and if so, spawns it through socket activation\n")
-		fmt.Printf("    daemon [--group=lxd] (default command)\n")
-		fmt.Printf("        Start the main LXD daemon\n")
-		fmt.Printf("    init [--auto] [--network-address=IP] [--network-port=8443] [--storage-backend=dir]\n")
-		fmt.Printf("         [--storage-create-device=DEVICE] [--storage-create-loop=SIZE] [--storage-pool=POOL]\n")
-		fmt.Printf("         [--trust-password=] [--preseed]\n")
-		fmt.Printf("        Setup storage and networking\n")
-		fmt.Printf("    ready\n")
-		fmt.Printf("        Tells LXD that any setup-mode configuration has been done and that it can start containers.\n")
-		fmt.Printf("    shutdown [--timeout=60]\n")
-		fmt.Printf("        Perform a clean shutdown of LXD and all running containers\n")
-		fmt.Printf("    waitready [--timeout=15]\n")
-		fmt.Printf("        Wait until LXD is ready to handle requests\n")
-		fmt.Printf("    import <container name> [--force]\n")
-		fmt.Printf("        Import a pre-existing container from storage\n")
-
-		fmt.Printf("\n\nCommon options:\n")
-		fmt.Printf("    --debug\n")
-		fmt.Printf("        Enable debug mode\n")
-		fmt.Printf("    --help\n")
-		fmt.Printf("        Print this help message\n")
-		fmt.Printf("    --logfile FILE\n")
-		fmt.Printf("        Logfile to log to (e.g., /var/log/lxd/lxd.log)\n")
-		fmt.Printf("    --syslog\n")
-		fmt.Printf("        Enable syslog logging\n")
-		fmt.Printf("    --verbose\n")
-		fmt.Printf("        Enable verbose mode\n")
-		fmt.Printf("    --version\n")
-		fmt.Printf("        Print LXD's version number and exit\n")
-
-		fmt.Printf("\nDaemon options:\n")
-		fmt.Printf("    --group GROUP\n")
-		fmt.Printf("        Group which owns the shared socket\n")
-
-		fmt.Printf("\nDaemon debug options:\n")
-		fmt.Printf("    --cpuprofile FILE\n")
-		fmt.Printf("        Enable cpu profiling into the specified file\n")
-		fmt.Printf("    --memprofile FILE\n")
-		fmt.Printf("        Enable memory profiling into the specified file\n")
-		fmt.Printf("    --print-goroutines-every SECONDS\n")
-		fmt.Printf("        For debugging, print a complete stack trace every n seconds\n")
-
-		fmt.Printf("\nInit options:\n")
-		fmt.Printf("    --auto\n")
-		fmt.Printf("        Automatic (non-interactive) mode\n")
-		fmt.Printf("    --preseed\n")
-		fmt.Printf("        Pre-seed mode, expects YAML config from stdin\n")
-
-		fmt.Printf("\nInit options for non-interactive mode (--auto):\n")
-		fmt.Printf("    --network-address ADDRESS\n")
-		fmt.Printf("        Address to bind LXD to (default: none)\n")
-		fmt.Printf("    --network-port PORT\n")
-		fmt.Printf("        Port to bind LXD to (default: 8443)\n")
-		fmt.Printf("    --storage-backend NAME\n")
-		fmt.Printf("        Storage backend to use (btrfs, dir, lvm or zfs, default: dir)\n")
-		fmt.Printf("    --storage-create-device DEVICE\n")
-		fmt.Printf("        Setup device based storage using DEVICE\n")
-		fmt.Printf("    --storage-create-loop SIZE\n")
-		fmt.Printf("        Setup loop based storage with SIZE in GB\n")
-		fmt.Printf("    --storage-pool NAME\n")
-		fmt.Printf("        Storage pool to use or create\n")
-		fmt.Printf("    --trust-password PASSWORD\n")
-		fmt.Printf("        Password required to add new clients\n")
-
-		fmt.Printf("\nShutdown options:\n")
-		fmt.Printf("    --timeout SECONDS\n")
-		fmt.Printf("        How long to wait before failing\n")
-
-		fmt.Printf("\nWaitready options:\n")
-		fmt.Printf("    --timeout SECONDS\n")
-		fmt.Printf("        How long to wait before failing\n")
-
-		fmt.Printf("\n\nInternal commands (don't call these directly):\n")
-		fmt.Printf("    forkexec\n")
-		fmt.Printf("        Execute a command in a container\n")
-		fmt.Printf("    forkgetnet\n")
-		fmt.Printf("        Get container network information\n")
-		fmt.Printf("    forkgetfile\n")
-		fmt.Printf("        Grab a file from a running container\n")
-		fmt.Printf("    forkmigrate\n")
-		fmt.Printf("        Restore a container after migration\n")
-		fmt.Printf("    forkputfile\n")
-		fmt.Printf("        Push a file to a running container\n")
-		fmt.Printf("    forkstart\n")
-		fmt.Printf("        Start a container\n")
-		fmt.Printf("    callhook\n")
-		fmt.Printf("        Call a container hook\n")
-		fmt.Printf("    migratedumpsuccess\n")
-		fmt.Printf("        Indicate that a migration dump was successful\n")
-		fmt.Printf("    netcat\n")
-		fmt.Printf("        Mirror a unix socket to stdin/stdout\n")
-	}
-
-	// Parse the arguments
-	gnuflag.Parse(true)
+	// Pupulate a new Args instance by parsing the command line arguments
+	// passed.
+	context := cmd.DefaultContext()
+	args := &Args{}
+	parser := cmd.NewParser(context, usage)
+	parser.Parse(os.Args, args)
 
 	// Set the global variables
-	debug = *argDebug
-	verbose = *argVerbose
+	debug = args.Debug
+	verbose = args.Verbose
 
-	if *argHelp {
-		// The user asked for help via --help, so we shouldn't print to
-		// stderr.
-		gnuflag.SetOut(os.Stdout)
-		gnuflag.Usage()
+	if args.Help {
+		context.Output(usage)
 		return nil
 	}
 
 	// Deal with --version right here
-	if *argVersion {
+	if args.Version {
 		fmt.Println(version.Version)
 		return nil
 	}
@@ -184,66 +63,63 @@ func run() error {
 
 	// Configure logging
 	syslog := ""
-	if *argSyslog {
+	if args.Syslog {
 		syslog = "lxd"
 	}
 
 	handler := eventsHandler{}
 	var err error
-	logger.Log, err = logging.GetLogger(syslog, *argLogfile, *argVerbose, *argDebug, handler)
+	logger.Log, err = logging.GetLogger(syslog, args.Logfile, args.Verbose, args.Debug, handler)
 	if err != nil {
 		fmt.Printf("%s", err)
 		return nil
 	}
 
 	// Process sub-commands
-	if len(os.Args) > 1 {
+	if args.Subcommand != "" {
 		// "forkputfile", "forkgetfile", "forkmount" and "forkumount" are handled specially in main_nsexec.go
 		// "forkgetnet" is partially handled in nsexec.go (setns)
-		switch os.Args[1] {
+		switch args.Subcommand {
 		// Main commands
 		case "activateifneeded":
 			return cmdActivateIfNeeded()
 		case "daemon":
-			return cmdDaemon()
+			return cmdDaemon(args)
 		case "callhook":
-			return cmdCallHook(os.Args[1:])
+			return cmdCallHook(args)
 		case "init":
-			return cmdInit()
+			return cmdInit(args)
 		case "ready":
 			return cmdReady()
 		case "shutdown":
-			return cmdShutdown()
+			return cmdShutdown(args)
 		case "waitready":
-			return cmdWaitReady()
+			return cmdWaitReady(args)
 		case "import":
-			return cmdImport(os.Args[1:])
+			return cmdImport(args)
 
 		// Internal commands
 		case "forkgetnet":
 			return cmdForkGetNet()
 		case "forkmigrate":
-			return cmdForkMigrate(os.Args[1:])
+			return cmdForkMigrate(args)
 		case "forkstart":
-			return cmdForkStart(os.Args[1:])
+			return cmdForkStart(args)
 		case "forkexec":
-			ret, err := cmdForkExec(os.Args[1:])
+			ret, err := cmdForkExec(args)
 			if err != nil {
 				fmt.Fprintf(os.Stderr, "error: %v\n", err)
 			}
 			os.Exit(ret)
 		case "netcat":
-			return cmdNetcat(os.Args[1:])
+			return cmdNetcat(args)
 		case "migratedumpsuccess":
-			return cmdMigrateDumpSuccess(os.Args[1:])
+			return cmdMigrateDumpSuccess(args)
 		}
+	} else {
+		return cmdDaemon(args) // Default subcommand
 	}
 
-	// Fail if some other command is passed
-	if gnuflag.NArg() > 0 {
-		gnuflag.Usage()
-		return fmt.Errorf("Unknown arguments")
-	}
-
-	return cmdDaemon()
+	context.Output(usage)
+	return fmt.Errorf("Unknown arguments")
 }
diff --git a/lxd/main_args.go b/lxd/main_args.go
new file mode 100644
index 000000000..006694c35
--- /dev/null
+++ b/lxd/main_args.go
@@ -0,0 +1,134 @@
+package main
+
+// Args contains all supported LXD command line flags.
+type Args struct {
+	Auto                 bool   `flag:"auto"`
+	Preseed              bool   `flag:"preseed"`
+	CPUProfile           string `flag:"cpuprofile"`
+	Debug                bool   `flag:"debug"`
+	Group                string `flag:"group"`
+	Help                 bool   `flag:"help"`
+	Logfile              string `flag:"logfile"`
+	MemProfile           string `flag:"memprofile"`
+	NetworkAddress       string `flag:"network-address"`
+	NetworkPort          int64  `flag:"network-port"`
+	PrintGoroutinesEvery int    `flag:"print-goroutines-every"`
+	StorageBackend       string `flag:"storage-backend"`
+	StorageCreateDevice  string `flag:"storage-create-device"`
+	StorageCreateLoop    int64  `flag:"storage-create-loop"`
+	StorageDataset       string `flag:"storage-pool"`
+	Syslog               bool   `flag:"syslog"`
+	Timeout              int    `flag:"timeout"`
+	TrustPassword        string `flag:"trust-password"`
+	Verbose              bool   `flag:"verbose"`
+	Version              bool   `flag:"version"`
+	Force                bool   `flag:"force"`
+
+	// The LXD subcommand, if any (e.g. "init" for "lxd init")
+	Subcommand string
+
+	// The subcommand parameters (e.g. []string{"foo"} for "lxd import foo").
+	Params []string
+
+	// Any extra arguments following the "--" separator.
+	Extra []string
+}
+
+const usage = `Usage: lxd [command] [options]
+
+Commands:
+    activateifneeded
+        Check if LXD should be started (at boot) and if so, spawns it through socket activation
+    daemon [--group=lxd] (default command)
+        Start the main LXD daemon
+    init [--auto] [--network-address=IP] [--network-port=8443] [--storage-backend=dir]
+         [--storage-create-device=DEVICE] [--storage-create-loop=SIZE] [--storage-pool=POOL]
+         [--trust-password=] [--preseed]
+        Setup storage and networking
+    ready
+        Tells LXD that any setup-mode configuration has been done and that it can start containers.
+    shutdown [--timeout=60]
+        Perform a clean shutdown of LXD and all running containers
+    waitready [--timeout=15]
+        Wait until LXD is ready to handle requests
+    import <container name> [--force]
+        Import a pre-existing container from storage
+
+
+Common options:
+    --debug
+        Enable debug mode
+    --help
+        Print this help message
+    --logfile FILE
+        Logfile to log to (e.g., /var/log/lxd/lxd.log)
+    --syslog
+        Enable syslog logging
+    --verbose
+        Enable verbose mode
+    --version
+        Print LXD's version number and exit
+
+Daemon options:
+    --group GROUP
+        Group which owns the shared socket
+
+Daemon debug options:
+    --cpuprofile FILE
+        Enable cpu profiling into the specified file
+    --memprofile FILE
+        Enable memory profiling into the specified file
+    --print-goroutines-every SECONDS
+        For debugging, print a complete stack trace every n seconds
+
+Init options:
+    --auto
+        Automatic (non-interactive) mode
+    --preseed
+        Pre-seed mode, expects YAML config from stdin
+
+Init options for non-interactive mode (--auto):
+    --network-address ADDRESS
+        Address to bind LXD to (default: none)
+    --network-port PORT
+        Port to bind LXD to (default: 8443)
+    --storage-backend NAME
+        Storage backend to use (btrfs, dir, lvm or zfs, default: dir)
+    --storage-create-device DEVICE
+        Setup device based storage using DEVICE
+    --storage-create-loop SIZE
+        Setup loop based storage with SIZE in GB
+    --storage-pool NAME
+        Storage pool to use or create
+    --trust-password PASSWORD
+        Password required to add new clients
+
+Shutdown options:
+    --timeout SECONDS
+        How long to wait before failing
+
+Waitready options:
+    --timeout SECONDS
+        How long to wait before failing
+
+
+Internal commands (don't call these directly):
+    forkexec
+        Execute a command in a container
+    forkgetnet
+        Get container network information
+    forkgetfile
+        Grab a file from a running container
+    forkmigrate
+        Restore a container after migration
+    forkputfile
+        Push a file to a running container
+    forkstart
+        Start a container
+    callhook
+        Call a container hook
+    migratedumpsuccess
+        Indicate that a migration dump was successful
+    netcat
+        Mirror a unix socket to stdin/stdout
+`
diff --git a/lxd/main_args_test.go b/lxd/main_args_test.go
new file mode 100644
index 000000000..bcb1fbc64
--- /dev/null
+++ b/lxd/main_args_test.go
@@ -0,0 +1,95 @@
+package main
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/lxc/lxd/shared/cmd"
+)
+
+// Check the default values of all command line arguments.
+func TestParse_ArgsDefaults(t *testing.T) {
+	context := cmd.NewMemoryContext(cmd.NewMemoryStreams(""))
+	line := []string{"lxd"}
+	args := &Args{}
+	parser := cmd.NewParser(context, "")
+	parser.Parse(line, args)
+
+	assert.Equal(t, false, args.Auto)
+	assert.Equal(t, false, args.Preseed)
+	assert.Equal(t, "", args.CPUProfile)
+	assert.Equal(t, false, args.Debug)
+	assert.Equal(t, "", args.Group)
+	assert.Equal(t, false, args.Help)
+	assert.Equal(t, "", args.Logfile)
+	assert.Equal(t, "", args.MemProfile)
+	assert.Equal(t, "", args.NetworkAddress)
+	assert.Equal(t, int64(-1), args.NetworkPort)
+	assert.Equal(t, -1, args.PrintGoroutinesEvery)
+	assert.Equal(t, "", args.StorageBackend)
+	assert.Equal(t, "", args.StorageCreateDevice)
+	assert.Equal(t, int64(-1), args.StorageCreateLoop)
+	assert.Equal(t, "", args.StorageDataset)
+	assert.Equal(t, false, args.Syslog)
+	assert.Equal(t, -1, args.Timeout)
+	assert.Equal(t, "", args.TrustPassword)
+	assert.Equal(t, false, args.Verbose)
+	assert.Equal(t, false, args.Version)
+	assert.Equal(t, false, args.Force)
+}
+
+// Check that parsing the command line results in the correct attributes
+// being set.
+func TestParse_ArgsCustom(t *testing.T) {
+	context := cmd.NewMemoryContext(cmd.NewMemoryStreams(""))
+	line := []string{
+		"lxd",
+		"--auto",
+		"--preseed",
+		"--cpuprofile", "lxd.cpu",
+		"--debug",
+		"--group", "lxd",
+		"--help",
+		"--logfile", "lxd.log",
+		"--memprofile", "lxd.mem",
+		"--network-address", "127.0.0.1",
+		"--network-port", "666",
+		"--print-goroutines-every", "10",
+		"--storage-backend", "btrfs",
+		"--storage-create-device", "/dev/sda2",
+		"--storage-create-loop", "8192",
+		"--storage-pool", "default",
+		"--syslog",
+		"--timeout", "30",
+		"--trust-password", "sekret",
+		"--verbose",
+		"--version",
+		"--force",
+	}
+	args := &Args{}
+	parser := cmd.NewParser(context, "")
+	parser.Parse(line, args)
+
+	assert.Equal(t, true, args.Auto)
+	assert.Equal(t, true, args.Preseed)
+	assert.Equal(t, "lxd.cpu", args.CPUProfile)
+	assert.Equal(t, true, args.Debug)
+	assert.Equal(t, "lxd", args.Group)
+	assert.Equal(t, true, args.Help)
+	assert.Equal(t, "lxd.log", args.Logfile)
+	assert.Equal(t, "lxd.mem", args.MemProfile)
+	assert.Equal(t, "127.0.0.1", args.NetworkAddress)
+	assert.Equal(t, int64(666), args.NetworkPort)
+	assert.Equal(t, 10, args.PrintGoroutinesEvery)
+	assert.Equal(t, "btrfs", args.StorageBackend)
+	assert.Equal(t, "/dev/sda2", args.StorageCreateDevice)
+	assert.Equal(t, int64(8192), args.StorageCreateLoop)
+	assert.Equal(t, "default", args.StorageDataset)
+	assert.Equal(t, true, args.Syslog)
+	assert.Equal(t, 30, args.Timeout)
+	assert.Equal(t, "sekret", args.TrustPassword)
+	assert.Equal(t, true, args.Verbose)
+	assert.Equal(t, true, args.Version)
+	assert.Equal(t, true, args.Force)
+}
diff --git a/lxd/main_callhook.go b/lxd/main_callhook.go
index c8507fd83..cc0b5c858 100644
--- a/lxd/main_callhook.go
+++ b/lxd/main_callhook.go
@@ -8,15 +8,15 @@ import (
 	"github.com/lxc/lxd/client"
 )
 
-func cmdCallHook(args []string) error {
+func cmdCallHook(args *Args) error {
 	// Parse the arguments
-	if len(args) < 4 {
+	if len(args.Params) < 3 {
 		return fmt.Errorf("Invalid arguments")
 	}
 
-	path := args[1]
-	id := args[2]
-	state := args[3]
+	path := args.Params[0]
+	id := args.Params[1]
+	state := args.Params[2]
 	target := ""
 
 	// Connect to LXD
diff --git a/lxd/main_daemon.go b/lxd/main_daemon.go
index 3aea04740..9bd9f10e7 100644
--- a/lxd/main_daemon.go
+++ b/lxd/main_daemon.go
@@ -13,14 +13,14 @@ import (
 	"github.com/lxc/lxd/shared/logger"
 )
 
-func cmdDaemon() error {
+func cmdDaemon(args *Args) error {
 	// Only root should run this
 	if os.Geteuid() != 0 {
 		return fmt.Errorf("This must be run as root")
 	}
 
-	if *argCPUProfile != "" {
-		f, err := os.Create(*argCPUProfile)
+	if args.CPUProfile != "" {
+		f, err := os.Create(args.CPUProfile)
 		if err != nil {
 			fmt.Printf("Error opening cpu profile file: %s\n", err)
 			return nil
@@ -29,8 +29,8 @@ func cmdDaemon() error {
 		defer pprof.StopCPUProfile()
 	}
 
-	if *argMemProfile != "" {
-		go memProfiler(*argMemProfile)
+	if args.MemProfile != "" {
+		go memProfiler(args.MemProfile)
 	}
 
 	neededPrograms := []string{"setfacl", "rsync", "tar", "unsquashfs", "xz"}
@@ -41,17 +41,17 @@ func cmdDaemon() error {
 		}
 	}
 
-	if *argPrintGoroutinesEvery > 0 {
+	if args.PrintGoroutinesEvery > 0 {
 		go func() {
 			for {
-				time.Sleep(time.Duration(*argPrintGoroutinesEvery) * time.Second)
+				time.Sleep(time.Duration(args.PrintGoroutinesEvery) * time.Second)
 				logger.Debugf(logger.GetStack())
 			}
 		}()
 	}
 
 	d := NewDaemon()
-	d.group = *argGroup
+	d.group = args.Group
 	d.SetupMode = shared.PathExists(shared.VarPath(".setup_mode"))
 	err := d.Init()
 	if err != nil {
diff --git a/lxd/main_forkexec.go b/lxd/main_forkexec.go
index bdbdcac64..ab1638a69 100644
--- a/lxd/main_forkexec.go
+++ b/lxd/main_forkexec.go
@@ -15,14 +15,17 @@ import (
 /*
  * This is called by lxd when called as "lxd forkexec <container>"
  */
-func cmdForkExec(args []string) (int, error) {
-	if len(args) < 6 {
-		return -1, fmt.Errorf("Bad arguments: %q", args)
+func cmdForkExec(args *Args) (int, error) {
+	if len(args.Params) < 3 {
+		return -1, fmt.Errorf("Bad params: %q", args.Params)
+	}
+	if len(args.Extra) < 1 {
+		return -1, fmt.Errorf("Bad extra: %q", args.Extra)
 	}
 
-	name := args[1]
-	lxcpath := args[2]
-	configPath := args[3]
+	name := args.Params[0]
+	lxcpath := args.Params[1]
+	configPath := args.Params[2]
 
 	c, err := lxc.NewContainer(name, lxcpath)
 	if err != nil {
@@ -63,7 +66,7 @@ func cmdForkExec(args []string) (int, error) {
 	cmd := []string{}
 
 	section := ""
-	for _, arg := range args[5:] {
+	for _, arg := range args.Extra {
 		// The "cmd" section must come last as it may contain a --
 		if arg == "--" && section != "cmd" {
 			section = ""
diff --git a/lxd/main_forkmigrate.go b/lxd/main_forkmigrate.go
index 45e437aae..ab3191c38 100644
--- a/lxd/main_forkmigrate.go
+++ b/lxd/main_forkmigrate.go
@@ -19,16 +19,16 @@ import (
  * want to fork for the same reasons we do forkstart (i.e. reduced memory
  * footprint when we fork tasks that will never free golang's memory, etc.)
  */
-func cmdForkMigrate(args []string) error {
-	if len(args) != 6 {
-		return fmt.Errorf("Bad arguments %q", args)
+func cmdForkMigrate(args *Args) error {
+	if len(args.Params) != 5 {
+		return fmt.Errorf("Bad arguments %q", args.Params)
 	}
 
-	name := args[1]
-	lxcpath := args[2]
-	configPath := args[3]
-	imagesDir := args[4]
-	preservesInodes, err := strconv.ParseBool(args[5])
+	name := args.Params[0]
+	lxcpath := args.Params[1]
+	configPath := args.Params[2]
+	imagesDir := args.Params[3]
+	preservesInodes, err := strconv.ParseBool(args.Params[4])
 
 	c, err := lxc.NewContainer(name, lxcpath)
 	if err != nil {
diff --git a/lxd/main_forkstart.go b/lxd/main_forkstart.go
index cc3d2ed1b..0c51daa1e 100644
--- a/lxd/main_forkstart.go
+++ b/lxd/main_forkstart.go
@@ -15,14 +15,14 @@ import (
  * 'forkstart' is used instead of just 'start' in the hopes that people
  * do not accidentally type 'lxd start' instead of 'lxc start'
  */
-func cmdForkStart(args []string) error {
-	if len(args) != 4 {
-		return fmt.Errorf("Bad arguments: %q", args)
+func cmdForkStart(args *Args) error {
+	if len(args.Params) != 3 {
+		return fmt.Errorf("Bad arguments: %q", args.Params)
 	}
 
-	name := args[1]
-	lxcpath := args[2]
-	configPath := args[3]
+	name := args.Params[0]
+	lxcpath := args.Params[1]
+	configPath := args.Params[2]
 
 	c, err := lxc.NewContainer(name, lxcpath)
 	if err != nil {
diff --git a/lxd/main_import.go b/lxd/main_import.go
index 8f0362370..842de6491 100644
--- a/lxd/main_import.go
+++ b/lxd/main_import.go
@@ -2,17 +2,18 @@ package main
 
 import (
 	"fmt"
+
 	"github.com/lxc/lxd/client"
 )
 
-func cmdImport(args []string) error {
-	if len(args) < 2 {
+func cmdImport(args *Args) error {
+	if len(args.Params) < 1 {
 		return fmt.Errorf("please specify a container to import")
 	}
-	name := args[1]
+	name := args.Params[0]
 	req := map[string]interface{}{
 		"name":  name,
-		"force": *argForce,
+		"force": args.Force,
 	}
 
 	c, err := lxd.ConnectLXDUnix("", nil)
diff --git a/lxd/main_init.go b/lxd/main_init.go
index 5fef9b401..5ed8da294 100644
--- a/lxd/main_init.go
+++ b/lxd/main_init.go
@@ -13,29 +13,17 @@ import (
 
 	"github.com/lxc/lxd/client"
 	"github.com/lxc/lxd/lxd/util"
+
 	"github.com/lxc/lxd/shared"
 	"github.com/lxc/lxd/shared/api"
 	"github.com/lxc/lxd/shared/cmd"
 	"github.com/lxc/lxd/shared/logger"
 )
 
-// CmdInitArgs holds command line arguments for the "lxd init" command.
-type CmdInitArgs struct {
-	Auto                bool
-	Preseed             bool
-	StorageBackend      string
-	StorageCreateDevice string
-	StorageCreateLoop   int64
-	StorageDataset      string
-	NetworkPort         int64
-	NetworkAddress      string
-	TrustPassword       string
-}
-
 // CmdInit implements the "lxd init" command line.
 type CmdInit struct {
 	Context         *cmd.Context
-	Args            *CmdInitArgs
+	Args            *Args
 	RunningInUserns bool
 	SocketPath      string
 	PasswordReader  func(int) ([]byte, error)
@@ -970,19 +958,8 @@ type cmdInitBridgeParams struct {
 // some change, and that are passed around as parameters.
 type reverter func() error
 
-func cmdInit() error {
+func cmdInit(args *Args) error {
 	context := cmd.NewContext(os.Stdin, os.Stdout, os.Stderr)
-	args := &CmdInitArgs{
-		Auto:                *argAuto,
-		Preseed:             *argPreseed,
-		StorageBackend:      *argStorageBackend,
-		StorageCreateDevice: *argStorageCreateDevice,
-		StorageCreateLoop:   *argStorageCreateLoop,
-		StorageDataset:      *argStorageDataset,
-		NetworkPort:         *argNetworkPort,
-		NetworkAddress:      *argNetworkAddress,
-		TrustPassword:       *argTrustPassword,
-	}
 	command := &CmdInit{
 		Context:         context,
 		Args:            args,
diff --git a/lxd/main_init_test.go b/lxd/main_init_test.go
index 551f251d2..98c2b3e0f 100644
--- a/lxd/main_init_test.go
+++ b/lxd/main_init_test.go
@@ -8,6 +8,7 @@ import (
 
 	"github.com/lxc/lxd/client"
 	"github.com/lxc/lxd/lxd/util"
+
 	"github.com/lxc/lxd/shared"
 	"github.com/lxc/lxd/shared/api"
 	"github.com/lxc/lxd/shared/cmd"
@@ -18,7 +19,7 @@ type cmdInitTestSuite struct {
 	lxdTestSuite
 	streams *cmd.MemoryStreams
 	context *cmd.Context
-	args    *CmdInitArgs
+	args    *Args
 	command *CmdInit
 	client  lxd.ContainerServer
 }
@@ -27,7 +28,7 @@ func (suite *cmdInitTestSuite) SetupTest() {
 	suite.lxdTestSuite.SetupTest()
 	suite.streams = cmd.NewMemoryStreams("")
 	suite.context = cmd.NewMemoryContext(suite.streams)
-	suite.args = &CmdInitArgs{
+	suite.args = &Args{
 		NetworkPort:       -1,
 		StorageCreateLoop: -1,
 	}
diff --git a/lxd/main_migratedumpsuccess.go b/lxd/main_migratedumpsuccess.go
index 56fce2e11..85603534b 100644
--- a/lxd/main_migratedumpsuccess.go
+++ b/lxd/main_migratedumpsuccess.go
@@ -8,9 +8,9 @@ import (
 	"github.com/lxc/lxd/shared/api"
 )
 
-func cmdMigrateDumpSuccess(args []string) error {
-	if len(args) != 3 {
-		return fmt.Errorf("bad migrate dump success args %s", args)
+func cmdMigrateDumpSuccess(args *Args) error {
+	if len(args.Params) != 2 {
+		return fmt.Errorf("bad migrate dump success args %s", args.Params)
 	}
 
 	c, err := lxd.ConnectLXDUnix("", nil)
@@ -18,14 +18,14 @@ func cmdMigrateDumpSuccess(args []string) error {
 		return err
 	}
 
-	url := fmt.Sprintf("%s/websocket?secret=%s", strings.TrimPrefix(args[1], "/1.0"), args[2])
+	url := fmt.Sprintf("%s/websocket?secret=%s", strings.TrimPrefix(args.Params[0], "/1.0"), args.Params[1])
 	conn, err := c.RawWebsocket(url)
 	if err != nil {
 		return err
 	}
 	conn.Close()
 
-	resp, _, err := c.RawQuery("GET", fmt.Sprintf("%s/wait", args[1]), nil, "")
+	resp, _, err := c.RawQuery("GET", fmt.Sprintf("%s/wait", args.Params[0]), nil, "")
 	if err != nil {
 		return err
 	}
diff --git a/lxd/main_netcat.go b/lxd/main_netcat.go
index 1cf9ee0a1..2fac5b338 100644
--- a/lxd/main_netcat.go
+++ b/lxd/main_netcat.go
@@ -18,12 +18,12 @@ import (
 // and does unbuffered netcatting of to socket to stdin/stdout. Any arguments
 // after the path to the unix socket are ignored, so that this can be passed
 // directly to rsync as the sync command.
-func cmdNetcat(args []string) error {
-	if len(args) < 3 {
-		return fmt.Errorf("Bad arguments %q", args)
+func cmdNetcat(args *Args) error {
+	if len(args.Params) < 2 {
+		return fmt.Errorf("Bad arguments %q", args.Params)
 	}
 
-	logPath := shared.LogPath(args[2], "netcat.log")
+	logPath := shared.LogPath(args.Params[1], "netcat.log")
 	if shared.PathExists(logPath) {
 		os.Remove(logPath)
 	}
@@ -34,15 +34,15 @@ func cmdNetcat(args []string) error {
 	}
 	defer logFile.Close()
 
-	uAddr, err := net.ResolveUnixAddr("unix", args[1])
+	uAddr, err := net.ResolveUnixAddr("unix", args.Params[0])
 	if err != nil {
-		logFile.WriteString(fmt.Sprintf("Could not resolve unix domain socket \"%s\": %s.\n", args[1], err))
+		logFile.WriteString(fmt.Sprintf("Could not resolve unix domain socket \"%s\": %s.\n", args.Params[0], err))
 		return err
 	}
 
 	conn, err := net.DialUnix("unix", nil, uAddr)
 	if err != nil {
-		logFile.WriteString(fmt.Sprintf("Could not dial unix domain socket \"%s\": %s.\n", args[1], err))
+		logFile.WriteString(fmt.Sprintf("Could not dial unix domain socket \"%s\": %s.\n", args.Params[0], err))
 		return err
 	}
 
@@ -52,7 +52,7 @@ func cmdNetcat(args []string) error {
 	go func() {
 		_, err := io.Copy(eagainWriter{os.Stdout}, eagainReader{conn})
 		if err != nil {
-			logFile.WriteString(fmt.Sprintf("Error while copying from stdout to unix domain socket \"%s\": %s.\n", args[1], err))
+			logFile.WriteString(fmt.Sprintf("Error while copying from stdout to unix domain socket \"%s\": %s.\n", args.Params[0], err))
 		}
 		conn.Close()
 		wg.Done()
@@ -61,7 +61,7 @@ func cmdNetcat(args []string) error {
 	go func() {
 		_, err := io.Copy(eagainWriter{conn}, eagainReader{os.Stdin})
 		if err != nil {
-			logFile.WriteString(fmt.Sprintf("Error while copying from unix domain socket \"%s\" to stdin: %s.\n", args[1], err))
+			logFile.WriteString(fmt.Sprintf("Error while copying from unix domain socket \"%s\" to stdin: %s.\n", args.Params[0], err))
 		}
 	}()
 
diff --git a/lxd/main_shutdown.go b/lxd/main_shutdown.go
index 7c982fbb7..00654ff64 100644
--- a/lxd/main_shutdown.go
+++ b/lxd/main_shutdown.go
@@ -7,7 +7,7 @@ import (
 	"github.com/lxc/lxd/client"
 )
 
-func cmdShutdown() error {
+func cmdShutdown(args *Args) error {
 	c, err := lxd.ConnectLXDUnix("", nil)
 	if err != nil {
 		return err
@@ -30,12 +30,12 @@ func cmdShutdown() error {
 		close(chMonitor)
 	}()
 
-	if *argTimeout > 0 {
+	if args.Timeout > 0 {
 		select {
 		case <-chMonitor:
 			break
-		case <-time.After(time.Second * time.Duration(*argTimeout)):
-			return fmt.Errorf("LXD still running after %ds timeout.", *argTimeout)
+		case <-time.After(time.Second * time.Duration(args.Timeout)):
+			return fmt.Errorf("LXD still running after %ds timeout.", args.Timeout)
 		}
 	} else {
 		<-chMonitor
diff --git a/lxd/main_waitready.go b/lxd/main_waitready.go
index 057abe758..3edca01cd 100644
--- a/lxd/main_waitready.go
+++ b/lxd/main_waitready.go
@@ -7,13 +7,13 @@ import (
 	"github.com/lxc/lxd/client"
 )
 
-func cmdWaitReady() error {
+func cmdWaitReady(args *Args) error {
 	var timeout int
 
-	if *argTimeout == -1 {
+	if args.Timeout == -1 {
 		timeout = 15
 	} else {
-		timeout = *argTimeout
+		timeout = args.Timeout
 	}
 
 	finger := make(chan error, 1)
diff --git a/shared/cmd/context.go b/shared/cmd/context.go
index ef192ce31..925152d82 100644
--- a/shared/cmd/context.go
+++ b/shared/cmd/context.go
@@ -6,6 +6,7 @@ import (
 	"gopkg.in/yaml.v2"
 	"io"
 	"io/ioutil"
+	"os"
 	"strconv"
 	"strings"
 
@@ -20,6 +21,12 @@ type Context struct {
 	stderr io.Writer
 }
 
+// DefaultContext returns a new Context connected the stdin, stdout and stderr
+// streams.
+func DefaultContext() *Context {
+	return NewContext(os.Stdin, os.Stderr, os.Stdout)
+}
+
 // NewContext creates a new command context with the given parameters.
 func NewContext(stdin io.Reader, stdout, stderr io.Writer) *Context {
 	return &Context{

From 68e016d7b34c99ae8244cb86cdce2cc84a0f52ff Mon Sep 17 00:00:00 2001
From: Free Ekanayaka <free.ekanayaka at canonical.com>
Date: Mon, 21 Aug 2017 12:30:03 +0000
Subject: [PATCH 3/4] Add a RunSubCommand helper for executing sub-commands

The new RubSubCommand helper executes LXD sub-commands, applying any common
setup needed.

Signed-off-by: Free Ekanayaka <free.ekanayaka at canonical.com>
---
 lxd/main_subcommand.go      | 99 +++++++++++++++++++++++++++++++++++++++++++++
 lxd/main_subcommand_test.go | 91 +++++++++++++++++++++++++++++++++++++++++
 shared/cmd/context.go       |  5 +++
 3 files changed, 195 insertions(+)
 create mode 100644 lxd/main_subcommand.go
 create mode 100644 lxd/main_subcommand_test.go

diff --git a/lxd/main_subcommand.go b/lxd/main_subcommand.go
new file mode 100644
index 000000000..03c9b75e1
--- /dev/null
+++ b/lxd/main_subcommand.go
@@ -0,0 +1,99 @@
+package main
+
+import (
+	"fmt"
+
+	log "gopkg.in/inconshreveable/log15.v2"
+
+	"github.com/lxc/lxd/shared"
+	"github.com/lxc/lxd/shared/cmd"
+	"github.com/lxc/lxd/shared/logger"
+	"github.com/lxc/lxd/shared/logging"
+	"github.com/lxc/lxd/shared/version"
+)
+
+// SubCommand is function that performs the logic of a specific LXD sub-command.
+type SubCommand func(*Args) error
+
+// SubCommandError implements the error interface and also carries with it an integer
+// exit code. If a Command returns an error of this kind, it will use its code
+// as exit status.
+type SubCommandError struct {
+	Code    int
+	Message string
+}
+
+func (e *SubCommandError) Error() string {
+	return e.Message
+}
+
+// SubCommandErrorf returns a new SubCommandError with the given code and the
+// given message (formatted with fmt.Sprintf).
+func SubCommandErrorf(code int, format string, a ...interface{}) *SubCommandError {
+	return &SubCommandError{
+		Code:    code,
+		Message: fmt.Sprintf(format, a...),
+	}
+}
+
+// RunSubCommand is the main entry point for all LXD subcommands, performing
+// common setup logic before firing up the subcommand.
+//
+// The ctx parameter provides input/output streams and related utilities, the
+// args one contains command line parameters, and handler is an additional
+// custom handler which will be added to the configured logger, along with the
+// default one (stderr) and the ones possibly installed by command line
+// arguments (via args.Syslog and args.Logfile).
+func RunSubCommand(command SubCommand, ctx *cmd.Context, args *Args, handler log.Handler) int {
+	// In case of --help or --version we just print the relevant output and
+	// return immediately
+	if args.Help {
+		ctx.Output(usage)
+		return 0
+	}
+	if args.Version {
+		ctx.Output("%s\n", version.Version)
+		return 0
+	}
+
+	// Run the setup code and, if successful, the command.
+	err := setupSubCommand(ctx, args, handler)
+	if err == nil {
+		err = command(args)
+	}
+	if err != nil {
+		code := 1
+		message := err.Error()
+		subCommandError, ok := err.(*SubCommandError)
+		if ok {
+			code = subCommandError.Code
+		}
+		if message != "" {
+			ctx.Error("error: %s\n", message)
+		}
+		return code
+	}
+	return 0
+}
+
+// Setup logic common across all LXD subcommands.
+func setupSubCommand(context *cmd.Context, args *Args, handler log.Handler) error {
+	// Check if LXD_DIR is valid.
+	if len(shared.VarPath("unix.sock")) > 107 {
+		return fmt.Errorf("LXD_DIR is too long, must be < %d", 107-len("unix.sock"))
+	}
+
+	// Configure logging.
+	syslog := ""
+	if args.Syslog {
+		syslog = "lxd"
+	}
+	var err error
+	logger.Log, err = logging.GetLogger(syslog, args.Logfile, args.Verbose, args.Debug, handler)
+	if err != nil {
+		context.Output("%s", err)
+		return err
+	}
+
+	return nil
+}
diff --git a/lxd/main_subcommand_test.go b/lxd/main_subcommand_test.go
new file mode 100644
index 000000000..561325751
--- /dev/null
+++ b/lxd/main_subcommand_test.go
@@ -0,0 +1,91 @@
+package main
+
+import (
+	"fmt"
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/lxc/lxd/shared/cmd"
+	"github.com/lxc/lxd/shared/version"
+)
+
+// If the help flag is set in the command line, the usage message is printed
+// and the runner exists without executing the command.
+func TestRunSubCommand_Help(t *testing.T) {
+	command := newFailingCommand(t)
+	ctx, streams := newSubCommandContext()
+	args := &Args{Help: true}
+
+	assert.Equal(t, 0, RunSubCommand(command, ctx, args, nil))
+	assert.Contains(t, streams.Out(), "Usage: lxd [command] [options]")
+}
+
+// If the version flag is set in the command line, the version is printed
+// and the runner exists without executing the command.
+func TestRunSubCommand_Version(t *testing.T) {
+	command := newFailingCommand(t)
+	ctx, streams := newSubCommandContext()
+	args := &Args{Version: true}
+
+	assert.Equal(t, 0, RunSubCommand(command, ctx, args, nil))
+	assert.Contains(t, streams.Out(), version.Version)
+}
+
+// If the path set in LXD_DIR is too long, an error is printed.
+func TestRunSubCommand_LxdDirTooLong(t *testing.T) {
+	// Restore original LXD_DIR.
+	if value, ok := os.LookupEnv("LXD_DIR"); ok {
+		defer os.Setenv("LXD_DIR", value)
+	} else {
+		defer os.Unsetenv("LXD_DIR")
+	}
+
+	os.Setenv("LXD_DIR", strings.Repeat("x", 200))
+
+	command := newFailingCommand(t)
+	ctx, streams := newSubCommandContext()
+	args := &Args{}
+
+	assert.Equal(t, 1, RunSubCommand(command, ctx, args, nil))
+	assert.Contains(t, streams.Err(), "error: LXD_DIR is too long")
+}
+
+// If the command being executed returns an error, it is printed on standard
+// err.
+func TestRunSubCommand_Error(t *testing.T) {
+	command := func(*Args) error { return fmt.Errorf("boom") }
+	ctx, streams := newSubCommandContext()
+	args := &Args{}
+
+	assert.Equal(t, 1, RunSubCommand(command, ctx, args, nil))
+	assert.Equal(t, "error: boom\n", streams.Err())
+}
+
+// If the command being executed returns a SubCommandError, RunSubCommand
+// returns the relevant status code.
+func TestRunSubCommand_SubCommandError(t *testing.T) {
+	command := func(*Args) error { return SubCommandErrorf(127, "") }
+	ctx, streams := newSubCommandContext()
+	args := &Args{}
+
+	assert.Equal(t, 127, RunSubCommand(command, ctx, args, nil))
+	assert.Equal(t, "", streams.Err())
+}
+
+// Create a new cmd.Context connected to in-memory input/output streams.
+func newSubCommandContext() (*cmd.Context, *cmd.MemoryStreams) {
+	streams := cmd.NewMemoryStreams("")
+	context := cmd.NewMemoryContext(streams)
+	return context, streams
+}
+
+// Return a command that makes the test fail if executed.
+func newFailingCommand(t *testing.T) SubCommand {
+	return func(*Args) error {
+		t.Fatal("unexpected command execution")
+		return nil
+	}
+}
diff --git a/shared/cmd/context.go b/shared/cmd/context.go
index 925152d82..0caa946b4 100644
--- a/shared/cmd/context.go
+++ b/shared/cmd/context.go
@@ -41,6 +41,11 @@ func (c *Context) Output(format string, a ...interface{}) {
 	fmt.Fprintf(c.stdout, format, a...)
 }
 
+// Error prints a message on standard error.
+func (c *Context) Error(format string, a ...interface{}) {
+	fmt.Fprintf(c.stderr, format, a...)
+}
+
 // AskBool asks a question an expect a yes/no answer.
 func (c *Context) AskBool(question string, defaultAnswer string) bool {
 	for {

From cbd04b32e10bebd4182ff0ed6cf491d713994299 Mon Sep 17 00:00:00 2001
From: Free Ekanayaka <free.ekanayaka at canonical.com>
Date: Tue, 29 Aug 2017 08:37:41 +0000
Subject: [PATCH 4/4] Plug RunSubCommand into main.go

Normalize sub-command invokation in main.go using the RunSubCommand
helper.

Signed-off-by: Free Ekanayaka <free.ekanayaka at canonical.com>
---
 lxd/main.go                  | 114 ++++++++++++-------------------------------
 lxd/main_activateifneeded.go |   2 +-
 lxd/main_forkexec.go         |  25 +++++-----
 lxd/main_forkgetnet.go       |   2 +-
 lxd/main_init.go             |   4 +-
 lxd/main_ready.go            |   2 +-
 6 files changed, 47 insertions(+), 102 deletions(-)

diff --git a/lxd/main.go b/lxd/main.go
index ffdc279a0..f8e78e150 100644
--- a/lxd/main.go
+++ b/lxd/main.go
@@ -1,16 +1,11 @@
 package main
 
 import (
-	"fmt"
 	"math/rand"
 	"os"
 	"time"
 
-	"github.com/lxc/lxd/shared"
 	"github.com/lxc/lxd/shared/cmd"
-	"github.com/lxc/lxd/shared/logger"
-	"github.com/lxc/lxd/shared/logging"
-	"github.com/lxc/lxd/shared/version"
 )
 
 // Global variables
@@ -28,13 +23,6 @@ func init() {
 }
 
 func main() {
-	if err := run(); err != nil {
-		fmt.Fprintf(os.Stderr, "error: %v\n", err)
-		os.Exit(1)
-	}
-}
-
-func run() error {
 	// Pupulate a new Args instance by parsing the command line arguments
 	// passed.
 	context := cmd.DefaultContext()
@@ -46,80 +34,40 @@ func run() error {
 	debug = args.Debug
 	verbose = args.Verbose
 
-	if args.Help {
-		context.Output(usage)
-		return nil
-	}
-
-	// Deal with --version right here
-	if args.Version {
-		fmt.Println(version.Version)
-		return nil
-	}
-
-	if len(shared.VarPath("unix.sock")) > 107 {
-		return fmt.Errorf("LXD_DIR is too long, must be < %d", 107-len("unix.sock"))
-	}
-
-	// Configure logging
-	syslog := ""
-	if args.Syslog {
-		syslog = "lxd"
+	// Process sub-commands
+	subcommand := cmdDaemon // default sub-command if none is specified
+	if args.Subcommand != "" {
+		subcommand, _ = subcommands[args.Subcommand]
 	}
-
-	handler := eventsHandler{}
-	var err error
-	logger.Log, err = logging.GetLogger(syslog, args.Logfile, args.Verbose, args.Debug, handler)
-	if err != nil {
-		fmt.Printf("%s", err)
-		return nil
+	if subcommand == nil {
+		context.Output(usage)
+		context.Error("error: Unknown arguments\n")
+		os.Exit(1)
 	}
 
-	// Process sub-commands
-	if args.Subcommand != "" {
-		// "forkputfile", "forkgetfile", "forkmount" and "forkumount" are handled specially in main_nsexec.go
-		// "forkgetnet" is partially handled in nsexec.go (setns)
-		switch args.Subcommand {
-		// Main commands
-		case "activateifneeded":
-			return cmdActivateIfNeeded()
-		case "daemon":
-			return cmdDaemon(args)
-		case "callhook":
-			return cmdCallHook(args)
-		case "init":
-			return cmdInit(args)
-		case "ready":
-			return cmdReady()
-		case "shutdown":
-			return cmdShutdown(args)
-		case "waitready":
-			return cmdWaitReady(args)
-		case "import":
-			return cmdImport(args)
+	os.Exit(RunSubCommand(subcommand, context, args, eventsHandler{}))
+}
 
-		// Internal commands
-		case "forkgetnet":
-			return cmdForkGetNet()
-		case "forkmigrate":
-			return cmdForkMigrate(args)
-		case "forkstart":
-			return cmdForkStart(args)
-		case "forkexec":
-			ret, err := cmdForkExec(args)
-			if err != nil {
-				fmt.Fprintf(os.Stderr, "error: %v\n", err)
-			}
-			os.Exit(ret)
-		case "netcat":
-			return cmdNetcat(args)
-		case "migratedumpsuccess":
-			return cmdMigrateDumpSuccess(args)
-		}
-	} else {
-		return cmdDaemon(args) // Default subcommand
-	}
+// Index of SubCommand functions by command line name
+//
+// "forkputfile", "forkgetfile", "forkmount" and "forkumount" are handled specially in main_nsexec.go
+// "forkgetnet" is partially handled in nsexec.go (setns)
+var subcommands = map[string]SubCommand{
+	// Main commands
+	"activateifneeded": cmdActivateIfNeeded,
+	"daemon":           cmdDaemon,
+	"callhook":         cmdCallHook,
+	"init":             cmdInit,
+	"ready":            cmdReady,
+	"shutdown":         cmdShutdown,
+	"waitready":        cmdWaitReady,
+	"import":           cmdImport,
 
-	context.Output(usage)
-	return fmt.Errorf("Unknown arguments")
+	// Internal commands
+	"forkgetnet":         cmdForkGetNet,
+	"forkmigrate":        cmdForkMigrate,
+	"forkstart":          cmdForkStart,
+	"forkexec":           cmdForkExec,
+	"netcat":             cmdNetcat,
+	"migratedumpsuccess": cmdMigrateDumpSuccess,
 }
diff --git a/lxd/main_activateifneeded.go b/lxd/main_activateifneeded.go
index d8e2d876f..592e04a30 100644
--- a/lxd/main_activateifneeded.go
+++ b/lxd/main_activateifneeded.go
@@ -10,7 +10,7 @@ import (
 	"github.com/lxc/lxd/shared/logger"
 )
 
-func cmdActivateIfNeeded() error {
+func cmdActivateIfNeeded(args *Args) error {
 	// Only root should run this
 	if os.Geteuid() != 0 {
 		return fmt.Errorf("This must be run as root")
diff --git a/lxd/main_forkexec.go b/lxd/main_forkexec.go
index ab1638a69..83d0e41a3 100644
--- a/lxd/main_forkexec.go
+++ b/lxd/main_forkexec.go
@@ -2,7 +2,6 @@ package main
 
 import (
 	"encoding/json"
-	"fmt"
 	"os"
 	"strings"
 	"syscall"
@@ -15,12 +14,12 @@ import (
 /*
  * This is called by lxd when called as "lxd forkexec <container>"
  */
-func cmdForkExec(args *Args) (int, error) {
+func cmdForkExec(args *Args) error {
 	if len(args.Params) < 3 {
-		return -1, fmt.Errorf("Bad params: %q", args.Params)
+		return SubCommandErrorf(-1, "Bad params: %q", args.Params)
 	}
 	if len(args.Extra) < 1 {
-		return -1, fmt.Errorf("Bad extra: %q", args.Extra)
+		return SubCommandErrorf(-1, "Bad extra: %q", args.Extra)
 	}
 
 	name := args.Params[0]
@@ -29,12 +28,12 @@ func cmdForkExec(args *Args) (int, error) {
 
 	c, err := lxc.NewContainer(name, lxcpath)
 	if err != nil {
-		return -1, fmt.Errorf("Error initializing container for start: %q", err)
+		return SubCommandErrorf(-1, "Error initializing container for start: %q", err)
 	}
 
 	err = c.LoadConfigFile(configPath)
 	if err != nil {
-		return -1, fmt.Errorf("Error opening startup config file: %q", err)
+		return SubCommandErrorf(-1, "Error opening startup config file: %q", err)
 	}
 
 	syscall.Dup3(int(os.Stdin.Fd()), 200, 0)
@@ -87,7 +86,7 @@ func cmdForkExec(args *Args) (int, error) {
 		} else if section == "cmd" {
 			cmd = append(cmd, arg)
 		} else {
-			return -1, fmt.Errorf("Invalid exec section: %s", section)
+			return SubCommandErrorf(-1, "Invalid exec section: %s", section)
 		}
 	}
 
@@ -95,7 +94,7 @@ func cmdForkExec(args *Args) (int, error) {
 
 	status, err := c.RunCommandNoWait(cmd, opts)
 	if err != nil {
-		return -1, fmt.Errorf("Failed running command: %q", err)
+		return SubCommandErrorf(-1, "Failed running command: %q", err)
 	}
 	// Send the PID of the executing process.
 	w := os.NewFile(uintptr(3), "attachedPid")
@@ -103,23 +102,23 @@ func cmdForkExec(args *Args) (int, error) {
 
 	err = json.NewEncoder(w).Encode(status)
 	if err != nil {
-		return -1, fmt.Errorf("Failed sending PID of executing command: %q", err)
+		return SubCommandErrorf(-1, "Failed sending PID of executing command: %q", err)
 	}
 
 	var ws syscall.WaitStatus
 	wpid, err := syscall.Wait4(status, &ws, 0, nil)
 	if err != nil || wpid != status {
-		return -1, fmt.Errorf("Failed finding process: %q", err)
+		return SubCommandErrorf(-1, "Failed finding process: %q", err)
 	}
 
 	if ws.Exited() {
-		return ws.ExitStatus(), nil
+		return SubCommandErrorf(ws.ExitStatus(), "")
 	}
 
 	if ws.Signaled() {
 		// 128 + n == Fatal error signal "n"
-		return 128 + int(ws.Signal()), nil
+		return SubCommandErrorf(128+int(ws.Signal()), "")
 	}
 
-	return -1, fmt.Errorf("Command failed")
+	return SubCommandErrorf(-1, "Command failed")
 }
diff --git a/lxd/main_forkgetnet.go b/lxd/main_forkgetnet.go
index 5ca9fdbf7..d541c0941 100644
--- a/lxd/main_forkgetnet.go
+++ b/lxd/main_forkgetnet.go
@@ -11,7 +11,7 @@ import (
 	"github.com/lxc/lxd/shared/api"
 )
 
-func cmdForkGetNet() error {
+func cmdForkGetNet(args *Args) error {
 	networks := map[string]api.ContainerStateNetwork{}
 
 	interfaces, err := net.Interfaces()
diff --git a/lxd/main_init.go b/lxd/main_init.go
index 5ed8da294..009ea41f5 100644
--- a/lxd/main_init.go
+++ b/lxd/main_init.go
@@ -3,7 +3,6 @@ package main
 import (
 	"fmt"
 	"net"
-	"os"
 	"os/exec"
 	"strconv"
 	"strings"
@@ -959,9 +958,8 @@ type cmdInitBridgeParams struct {
 type reverter func() error
 
 func cmdInit(args *Args) error {
-	context := cmd.NewContext(os.Stdin, os.Stdout, os.Stderr)
 	command := &CmdInit{
-		Context:         context,
+		Context:         cmd.DefaultContext(),
 		Args:            args,
 		RunningInUserns: shared.RunningInUserNS(),
 		SocketPath:      "",
diff --git a/lxd/main_ready.go b/lxd/main_ready.go
index fb4f0dfe5..98f8ba634 100644
--- a/lxd/main_ready.go
+++ b/lxd/main_ready.go
@@ -4,7 +4,7 @@ import (
 	"github.com/lxc/lxd/client"
 )
 
-func cmdReady() error {
+func cmdReady(args *Args) error {
 	c, err := lxd.ConnectLXDUnix("", nil)
 	if err != nil {
 		return err


More information about the lxc-devel mailing list