[lxc-devel] [lxd/stable-2.0] WIP: backport "lxd init" code structure changes
freeekanayaka on Github
lxc-bot at linuxcontainers.org
Thu Aug 31 08:34:57 UTC 2017
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 301 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20170831/96519b04/attachment.bin>
-------------- next part --------------
From 9649df9247adf2dbac2813a63510d3b906f0909a Mon Sep 17 00:00:00 2001
From: Free Ekanayaka <free.ekanayaka at canonical.com>
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 <free.ekanayaka at canonical.com>
---
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) {
More information about the lxc-devel
mailing list