The following pull request was submitted through Github. It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/3745
This e-mail was sent by the LXC bot, direct replies will not reach the author unless they happen to be subscribed to this list. === Description (from pull-request) ===
From 9649df9247adf2dbac2813a63510d3b906f0909a Mon Sep 17 00:00:00 2001 From: Free Ekanayaka <[email protected]> Date: Thu, 4 May 2017 14:42:35 +0200 Subject: [PATCH] Extract validation of --auto args into a separate method This just mechanically moves into a standalone method the code that validates the various arguments that go with --auto. It makes the rest of the code a bit shorter and easier to refactor in the follow-up commit. For good measure, I've also added a few unit tests covering some of the invalid arguments combinations or values. Signed-off-by: Free Ekanayaka <[email protected]> --- lxd/main_init.go | 160 +++++++++++++++++++++++++++++++------------------- lxd/main_init_test.go | 116 ++++++++++++++++++++++++++++++++++-- 2 files changed, 213 insertions(+), 63 deletions(-) diff --git a/lxd/main_init.go b/lxd/main_init.go index e673f9924..19d1cd72a 100644 --- a/lxd/main_init.go +++ b/lxd/main_init.go @@ -37,6 +37,39 @@ type CmdInit struct { // Run triggers the execution of the init command. func (cmd *CmdInit) Run() error { + // Figure what storage drivers among the supported ones are actually + // available on this system. + availableStoragePoolsDrivers := cmd.availableStoragePoolsDrivers() + + // Check that command line arguments don't conflict with each other + err := cmd.validateArgs() + if err != nil { + return err + } + + // Connect to LXD + client, err := lxd.ConnectLXDUnix(cmd.SocketPath, nil) + if err != nil { + return fmt.Errorf("Unable to talk to LXD: %s", err) + } + + err = cmd.runAutoOrInteractive(client, availableStoragePoolsDrivers) + + if err == nil { + cmd.Context.Output("LXD has been successfully configured.\n") + } + + return err +} + +// Run the logic for auto or interactive mode. +// +// XXX: this logic is going to be refactored into two separate runAuto +// and runInteractive methods, sharing relevant logic with +// runPreseed. The idea being that both runAuto and runInteractive +// will end up populating the same low-level cmdInitData structure +// passed to the common run() method. +func (cmd *CmdInit) runAutoOrInteractive(c lxd.ContainerServer, backendsAvailable []string) error { var defaultPrivileged int // controls whether we set security.privileged=true var storageBackend string // dir or zfs var storageMode string // existing, loop or device @@ -51,26 +84,6 @@ func (cmd *CmdInit) Run() error { defaultPrivileged = -1 runningInUserns = cmd.RunningInUserns - backendsAvailable := []string{"dir"} - backendsSupported := []string{"dir", "zfs"} - - // Detect zfs - out, err := exec.LookPath("zfs") - if err == nil && len(out) != 0 && !runningInUserns { - _ = loadModule("zfs") - - _, err := shared.RunCommand("zpool", "list") - if err == nil { - backendsAvailable = append(backendsAvailable, "zfs") - } - } - - // Connect to LXD - c, err := lxd.ConnectLXDUnix(cmd.SocketPath, nil) - if err != nil { - return fmt.Errorf("Unable to talk to LXD: %s", err) - } - // Check that we have no containers or images in the store containers, err := c.GetContainerNames() if err != nil { @@ -91,38 +104,9 @@ func (cmd *CmdInit) Run() error { cmd.Args.StorageBackend = "dir" } - // Do a bunch of sanity checks - if !shared.StringInSlice(cmd.Args.StorageBackend, backendsSupported) { - return fmt.Errorf("The requested backend '%s' isn't supported by lxd init.", cmd.Args.StorageBackend) - } - - if !shared.StringInSlice(cmd.Args.StorageBackend, backendsAvailable) { - return fmt.Errorf("The requested backend '%s' isn't available on your system (missing tools).", cmd.Args.StorageBackend) - } - - if cmd.Args.StorageBackend == "dir" { - if cmd.Args.StorageCreateLoop != -1 || cmd.Args.StorageCreateDevice != "" || cmd.Args.StoragePool != "" { - return fmt.Errorf("None of --storage-pool, --storage-create-device or --storage-create-loop may be used with the 'dir' backend.") - } - } - - if cmd.Args.StorageBackend == "zfs" { - if cmd.Args.StorageCreateLoop != -1 && cmd.Args.StorageCreateDevice != "" { - return fmt.Errorf("Only one of --storage-create-device or --storage-create-loop can be specified with the 'zfs' backend.") - } - - if cmd.Args.StoragePool == "" { - return fmt.Errorf("--storage-pool must be specified with the 'zfs' backend.") - } - } - - if cmd.Args.NetworkAddress == "" { - if cmd.Args.NetworkPort != -1 { - return fmt.Errorf("--network-port cannot be used without --network-address.") - } - if cmd.Args.TrustPassword != "" { - return fmt.Errorf("--trust-password cannot be used without --network-address.") - } + err = cmd.validateArgsAuto(backendsAvailable) + if err != nil { + return err } // Set the local variables @@ -142,18 +126,14 @@ func (cmd *CmdInit) Run() error { networkPort = cmd.Args.NetworkPort trustPassword = cmd.Args.TrustPassword } else { - if cmd.Args.StorageBackend != "" || cmd.Args.StorageCreateDevice != "" || cmd.Args.StorageCreateLoop != -1 || cmd.Args.StoragePool != "" || cmd.Args.NetworkAddress != "" || cmd.Args.NetworkPort != -1 || cmd.Args.TrustPassword != "" { - return fmt.Errorf("Init configuration is only valid with --auto") - } - defaultStorage := "dir" if shared.StringInSlice("zfs", backendsAvailable) { defaultStorage = "zfs" } - storageBackend = cmd.Context.AskChoice(fmt.Sprintf("Name of the storage backend to use (dir or zfs) [default=%s]: ", defaultStorage), backendsSupported, defaultStorage) + storageBackend = cmd.Context.AskChoice(fmt.Sprintf("Name of the storage backend to use (dir or zfs) [default=%s]: ", defaultStorage), supportedStoragePoolDrivers, defaultStorage) - if !shared.StringInSlice(storageBackend, backendsSupported) { + if !shared.StringInSlice(storageBackend, supportedStoragePoolDrivers) { return fmt.Errorf("The requested backend '%s' isn't supported by lxd init.", storageBackend) } @@ -329,10 +309,70 @@ they otherwise would. } } - fmt.Printf("LXD has been successfully configured.\n") + cmd.Context.Output("LXD has been successfully configured.\n") + return nil +} + +// Check that the arguments passed via command line are consistent, +// and no invalid combination is provided. +func (cmd *CmdInit) validateArgs() error { + if !cmd.Args.Auto { + if cmd.Args.StorageBackend != "" || cmd.Args.StorageCreateDevice != "" || cmd.Args.StorageCreateLoop != -1 || cmd.Args.StoragePool != "" || cmd.Args.NetworkAddress != "" || cmd.Args.NetworkPort != -1 || cmd.Args.TrustPassword != "" { + return fmt.Errorf("Init configuration is only valid with --auto") + } + } + return nil +} + +// Check that the arguments passed along with --auto are valid and consistent. +// and no invalid combination is provided. +func (cmd *CmdInit) validateArgsAuto(availableStoragePoolsDrivers []string) error { + if !shared.StringInSlice(cmd.Args.StorageBackend, supportedStoragePoolDrivers) { + return fmt.Errorf("The requested backend '%s' isn't supported by lxd init.", cmd.Args.StorageBackend) + } + if !shared.StringInSlice(cmd.Args.StorageBackend, availableStoragePoolsDrivers) { + return fmt.Errorf("The requested backend '%s' isn't available on your system (missing tools).", cmd.Args.StorageBackend) + } + + if cmd.Args.StorageBackend == "dir" { + if cmd.Args.StorageCreateLoop != -1 || cmd.Args.StorageCreateDevice != "" || cmd.Args.StoragePool != "" { + return fmt.Errorf("None of --storage-pool, --storage-create-device or --storage-create-loop may be used with the 'dir' backend.") + } + } else { + if cmd.Args.StorageCreateLoop != -1 && cmd.Args.StorageCreateDevice != "" { + return fmt.Errorf("Only one of --storage-create-device or --storage-create-loop can be specified.") + } + } + + if cmd.Args.NetworkAddress == "" { + if cmd.Args.NetworkPort != -1 { + return fmt.Errorf("--network-port cannot be used without --network-address.") + } + if cmd.Args.TrustPassword != "" { + return fmt.Errorf("--trust-password cannot be used without --network-address.") + } + } + return nil } +// Return the available storage pools drivers (depending on installed tools). +func (cmd *CmdInit) availableStoragePoolsDrivers() []string { + drivers := []string{"dir"} + + // Detect zfs + out, err := exec.LookPath("zfs") + if err == nil && len(out) != 0 && !cmd.RunningInUserns { + _ = loadModule("zfs") + + _, err := shared.RunCommand("zpool", "list") + if err == nil { + drivers = append(drivers, "zfs") + } + } + return drivers +} + func (cmd *CmdInit) setServerConfig(c lxd.ContainerServer, key string, value string) error { server, etag, err := c.GetServer() if err != nil { @@ -419,3 +459,5 @@ func cmdInit() error { } return command.Run() } + +var supportedStoragePoolDrivers = []string{"dir", "zfs"} diff --git a/lxd/main_init_test.go b/lxd/main_init_test.go index 0c4b0bdc8..53da4e9e2 100644 --- a/lxd/main_init_test.go +++ b/lxd/main_init_test.go @@ -3,20 +3,24 @@ package main import ( "testing" + lxd "github.com/lxc/lxd/client" "github.com/lxc/lxd/shared/cmd" "github.com/stretchr/testify/suite" ) type cmdInitTestSuite struct { lxdTestSuite + streams *cmd.MemoryStreams context *cmd.Context args *CmdInitArgs command *CmdInit + client lxd.ContainerServer } -func (suite *cmdInitTestSuite) SetupSuite() { - suite.lxdTestSuite.SetupSuite() - suite.context = cmd.NewMemoryContext(cmd.NewMemoryStreams("")) +func (suite *cmdInitTestSuite) SetupTest() { + suite.lxdTestSuite.SetupTest() + suite.streams = cmd.NewMemoryStreams("") + suite.context = cmd.NewMemoryContext(suite.streams) suite.args = &CmdInitArgs{ NetworkPort: -1, StorageCreateLoop: -1, @@ -27,6 +31,9 @@ func (suite *cmdInitTestSuite) SetupSuite() { RunningInUserns: false, SocketPath: suite.d.UnixSocket.Socket.Addr().String(), } + client, err := lxd.ConnectLXDUnix(suite.command.SocketPath, nil) + suite.Req.Nil(err) + suite.client = client } // If any argument intended for --auto is passed in interactive mode, an @@ -34,7 +41,108 @@ func (suite *cmdInitTestSuite) SetupSuite() { func (suite *cmdInitTestSuite) TestCmdInit_InteractiveWithAutoArgs() { suite.args.NetworkPort = 9999 err := suite.command.Run() - suite.Req.Equal(err.Error(), "Init configuration is only valid with --auto") + suite.Req.Equal("Init configuration is only valid with --auto", err.Error()) +} + +// Some arguments can only be passed together with --auto. +func (suite *cmdInitTestSuite) TestCmdInit_AutoSpecificArgs() { + suite.args.StorageBackend = "dir" + err := suite.command.Run() + suite.Req.Equal("Init configuration is only valid with --auto", err.Error()) +} + +// The images auto-update interval can be interactively set by simply accepting +// the answer "yes" to the relevant question. +func (suite *cmdInitTestSuite) TestCmdInit_ImagesAutoUpdateAnswerYes() { + answers := &cmdInitAnswers{ + WantImageAutoUpdate: true, + } + answers.Render(suite.streams) + + suite.Req.Nil(suite.command.Run()) + + key, _ := daemonConfig["images.auto_update_interval"] + suite.Req.Equal("6", key.Get()) +} + +// If the images auto-update interval value is already set to non-zero, it +// won't be overwritten. +func (suite *cmdInitTestSuite) TestCmdInit_ImagesAutoUpdateNoOverwrite() { + key, _ := daemonConfig["images.auto_update_interval"] + err := key.Set(suite.d, "10") + suite.Req.Nil(err) + + answers := &cmdInitAnswers{ + WantImageAutoUpdate: true, + } + answers.Render(suite.streams) + + suite.Req.Nil(suite.command.Run()) + + suite.Req.Equal("10", key.Get()) +} + +// If an invalid backend type is passed with --storage-backend, an +// error is returned. +func (suite *cmdInitTestSuite) TestCmdInit_AutoWithInvalidBackendType() { + suite.args.Auto = true + suite.args.StorageBackend = "foo" + + err := suite.command.Run() + suite.Req.Equal("The requested backend 'foo' isn't supported by lxd init.", err.Error()) +} + +// If an backend type that is not available on the system is passed +// with --storage-backend, an error is returned. +func (suite *cmdInitTestSuite) TestCmdInit_AutoWithUnavailableBackendType() { + suite.args.Auto = true + suite.args.StorageBackend = "zfs" + suite.command.RunningInUserns = true // This makes zfs unavailable + + err := suite.command.Run() + suite.Req.Equal("The requested backend 'zfs' isn't available on your system (missing tools).", err.Error()) +} + +// If --storage-backend is set to "dir", --storage-create-device can't be passed. +func (suite *cmdInitTestSuite) TestCmdInit_AutoWithDirStorageBackendAndCreateDevice() { + suite.args.Auto = true + suite.args.StorageBackend = "dir" + suite.args.StorageCreateDevice = "/dev/sda4" + + err := suite.command.Run() + suite.Req.Equal("None of --storage-pool, --storage-create-device or --storage-create-loop may be used with the 'dir' backend.", err.Error()) +} + +// Convenience for building the input text a user would enter for a certain +// sequence of answers. +type cmdInitAnswers struct { + WantStoragePool bool + WantAvailableOverNetwork bool + BindToAddress string + BindToPort string + WantImageAutoUpdate bool + WantNetworkBridge bool + BridgeName string + BridgeIPv4 string + BridgeIPv6 string +} + +// Render the input text the user would type for the desired answers, populating +// the stdin of the given streams. +func (answers *cmdInitAnswers) Render(streams *cmd.MemoryStreams) { + streams.InputAppendBoolAnswer(answers.WantStoragePool) + streams.InputAppendBoolAnswer(answers.WantAvailableOverNetwork) + if answers.WantAvailableOverNetwork { + streams.InputAppendLine(answers.BindToAddress) + streams.InputAppendLine(answers.BindToPort) + } + streams.InputAppendBoolAnswer(answers.WantImageAutoUpdate) + streams.InputAppendBoolAnswer(answers.WantNetworkBridge) + if answers.WantNetworkBridge { + streams.InputAppendLine(answers.BridgeName) + streams.InputAppendLine(answers.BridgeIPv4) + streams.InputAppendLine(answers.BridgeIPv6) + } } func TestCmdInitTestSuite(t *testing.T) {
_______________________________________________ lxc-devel mailing list [email protected] http://lists.linuxcontainers.org/listinfo/lxc-devel
